xfs_repair: clean up process_sf_dir2

entry #1 is zero length in shortform dir 67, resetting to 29 entry contains illegal character in shortform dir 67 junking entry "bbbbbbbbbbbbbbbbbbbbbbbb" in directory inode 67

There are two "fix sfep->namelen" cases, but we only mark *dino_dirty = 1 in one of them. Add the other to ensure that the change gets written out.

Signed-off-by Eric Sandeen <[email protected]>

---

repair/dir2.c +1 -0

index 6b8964d..25793e9 100644
@@ -881,6 +881,7 @@ _("entry \"%*.*s\" in shortform directory %" PRIu64 " references %s inode %" PRI
 _("zero length entry in shortform dir %" PRIu64 ", resetting to %d\n"),
 						ino, namelen);
 					sfep->namelen = namelen;
+					*dino_dirty = 1;
 				} else  {
 					do_warn(
 _("zero length entry in shortform dir %" PRIu64 ", would set to %d\n"),

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

We're testing for an impossible case in here:

                if (i == num_entries - 1)  {
...
                } else  {
...
                                if (i == num_entries - 1)
                                    /* can't happen! */
...
                }

So, remove the impossible case.

Signed-off-by Eric Sandeen <[email protected]>

---

repair/dir2.c +6 -20

index 25793e9..7aede6a 100644
@@ -928,26 +928,12 @@ _("size of last entry overflows space left in in shortform dir %" PRIu64 ", "),
 				do_warn(
 _("size of entry #%d overflows space left in in shortform dir %" PRIu64 "\n"),
 					i, ino);
-				if (!no_modify)  {
-					if (i == num_entries - 1)
-						do_warn(
-						_("junking entry #%d\n"),
-							i);
-					else
-						do_warn(
-						_("junking %d entries\n"),
-							num_entries - i);
-				} else  {
-					if (i == num_entries - 1)
-						do_warn(
-						_("would junk entry #%d\n"),
-							i);
-					else
-						do_warn(
-						_("would junk %d entries\n"),
-							num_entries - i);
-				}
-
+				if (!no_modify)
+					do_warn(_("junking %d entries\n"),
+						num_entries - i);
+				else
+					do_warn(_("would junk %d entries\n"),
+						num_entries - i);
 				break;
 			}
 		}

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

process_sf_dir2() has 2 cases for a bad namelen: on-disk namelen is 0, or on-disk namelen extends beyond the directory i_size.

After the prior 2 patches, the code for each case is now essentially the same, so collapse the two cases.

Note, this does slightly change some of the error messages.

Signed-off-by Eric Sandeen <[email protected]>

---

repair/dir2.c +4 -9

index 7aede6a..5512e41 100644
@@ -861,36 +861,44 @@ process_sf_dir2(
 _("entry \"%*.*s\" in shortform directory %" PRIu64 " references %s inode %" PRIu64 "\n"),
 				namelen, namelen, sfep->name, ino, junkreason,
 				lino);
-		if (namelen == 0)  {
-			/*
-			 * if we're really lucky, this is
-			 * the last entry in which case we
-			 * can use the dir size to set the
-			 * namelen value.  otherwise, forget
-			 * it because we're not going to be
-			 * able to find the next entry.
-			 */
+
+		/* is dir namelen 0 or does this entry extend past dir size? */
+		if (namelen == 0) {
+			junkreason = _("is zero length");
 			bad_sfnamelen = 1;

repair/dir2.c +10 -39

+			 * case we can use the dir size to set the namelen
+			 * value.  otherwise, forget it because we're not going
+			 * to be able to find the next entry.
+			 */
 			if (i == num_entries - 1)  {
 				namelen = ino_dir_size -
 					((__psint_t) &sfep->name[0] -
 					 (__psint_t) sfp);
 				if (!no_modify)  {
 					do_warn(
-_("zero length entry in shortform dir %" PRIu64 ", resetting to %d\n"),
-						ino, namelen);
+_("entry #%d %s in shortform dir %" PRIu64 ", resetting to %d\n"),
+						i, junkreason, ino, namelen);
 					sfep->namelen = namelen;
 					*dino_dirty = 1;
 				} else  {
 					do_warn(
-_("zero length entry in shortform dir %" PRIu64 ", would set to %d\n"),
-						ino, namelen);
+_("entry #%d %s in shortform dir %" PRIu64 ", would set to %d\n"),
+						i, junkreason, ino, namelen);
 				}
 			} else  {
 				do_warn(
-_("zero length entry in shortform dir %" PRIu64 ""),
-					ino);
+_("entry #%d %s in shortform dir %" PRIu64),
+					i, junkreason, ino);
 				if (!no_modify)
 					do_warn(_(", junking %d entries\n"),
 						num_entries - i);

@@ -903,39 +911,6 @@ _("zero length entry in shortform dir %" PRIu64 ""),
 				 */
 				break;
 			}
-		} else if ((__psint_t) sfep - (__psint_t) sfp +
-				xfs_dir3_sf_entsize(mp, sfp, sfep->namelen)
-							> ino_dir_size)  {
-			bad_sfnamelen = 1;
-
-			if (i == num_entries - 1)  {
-				namelen = ino_dir_size -
-					((__psint_t) &sfep->name[0] -
-					 (__psint_t) sfp);
-				do_warn(
-_("size of last entry overflows space left in in shortform dir %" PRIu64 ", "),
-					ino);
-				if (!no_modify)  {
-					do_warn(_("resetting to %d\n"),
-						namelen);
-					sfep->namelen = namelen;
-					*dino_dirty = 1;
-				} else  {
-					do_warn(_("would reset to %d\n"),
-						namelen);
-				}
-			} else  {
-				do_warn(
-_("size of entry #%d overflows space left in in shortform dir %" PRIu64 "\n"),
-					i, ino);
-				if (!no_modify)
-					do_warn(_("junking %d entries\n"),
-						num_entries - i);
-				else
-					do_warn(_("would junk %d entries\n"),
-						num_entries - i);
-				break;
-			}
 		}
 
 		/*

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

process_sf_dir2() tries to special-case the last entry in a short form dir, and salvage it if the name length is wrong by using the dir size as a clue to what the length should be.

However, this doesn't actually work; there is either a 32-bit or 64-bit inode after the name, and with dirv3 there's a file type in there as well. Extending the name length to the dir size means it overlaps these fields, which often have a 0 in the top bits, and then namecheck() fails the result and junks it anyway:

Quoted 3 lines

entry #1 is zero length in shortform dir 67, resetting to 29 entry contains illegal character in shortform dir 67 junking entry "bbbbbbbbbbbbbbbbbbbbbbbb" in directory inode 67

And depending on the corruption, the current code will set a new negative namelen if it turns out that the name itself starts beyond dir size; it can't be shortened enough.

So, we could fix this up, and choose a new namelen such that the xfs_dir2_ino8_t and/or xfs_dir2_ino8_t and/or file type still fits at the end, but we really seem to be reaching the point of diminishing returns. The chances that nothing but the name length is wrong, and that the remaining few bytes at the end of the dir size are actually correct, seems awfully small.

Therefore just remove this special case, which I think is of questionable value.

Signed-off-by Eric Sandeen <[email protected]>

---

repair/dir2.c +12 -35

index 5512e41..b1816f4 100644
@@ -874,43 +874,20 @@ _("entry \"%*.*s\" in shortform directory %" PRIu64 " references %s inode %" PRI
 		}
 
 		if (bad_sfnamelen) {
+			do_warn(
+_("entry #%d %s in shortform dir %" PRIu64),
+				i, junkreason, ino);
+			if (!no_modify)
+				do_warn(_(", junking %d entries\n"),
+					num_entries - i);
+			else
+				do_warn(_(", would junk %d entries\n"),
+					num_entries - i);
 			/*
-			 * if we're really lucky, this is the last entry in
-			 * case we can use the dir size to set the namelen
-			 * value.  otherwise, forget it because we're not going
-			 * to be able to find the next entry.
+			 * don't process the rest of the directory,
+			 * break out of processing loop
 			 */
-			if (i == num_entries - 1)  {
-				namelen = ino_dir_size -
-					((__psint_t) &sfep->name[0] -
-					 (__psint_t) sfp);
-				if (!no_modify)  {
-					do_warn(
-_("entry #%d %s in shortform dir %" PRIu64 ", resetting to %d\n"),
-						i, junkreason, ino, namelen);
-					sfep->namelen = namelen;
-					*dino_dirty = 1;
-				} else  {
-					do_warn(
-_("entry #%d %s in shortform dir %" PRIu64 ", would set to %d\n"),
-						i, junkreason, ino, namelen);
-				}
-			} else  {
-				do_warn(
-_("entry #%d %s in shortform dir %" PRIu64),
-					i, junkreason, ino);
-				if (!no_modify)
-					do_warn(_(", junking %d entries\n"),
-						num_entries - i);
-				else
-					do_warn(_(", would junk %d entries\n"),
-						num_entries - i);
-				/*
-				 * don't process the rest of the directory,
-				 * break out of processing looop
-				 */
-				break;
-			}
+			break;
 		}
 
 		/*

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs