On 1/3/19 3:20 PM, Dave Chinner wrote: > On Thu, Jan 03, 2019 at 01:15:56PM -0600, Eric Sandeen wrote: >> For some reason, since the earliest days of XFS, a '/' character >> in an extended attribute name has been treated as corruption by >> xfs_repair. This despite nothing in other userspace tools or the >> kernel having this restriction. >> >> My best guess is that this was an unintentional leftover from >> common code between dirs & attrs in the "da" code, and there has >> never been a good reason for it. >> >> Since userspace and kernelspace allow such a name to be set, >> listed, and read, it seems wrong to flag it as corruption. >> So, make this test conditional on whether we're validating a name >> in a dir, as opposed to the name of an attr. > > Sounds fair. > >> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx> >> --- >> >> >> diff --git a/repair/attr_repair.c b/repair/attr_repair.c >> index 1d04500..2f6f7ef 100644 >> --- a/repair/attr_repair.c >> +++ b/repair/attr_repair.c >> @@ -292,11 +292,9 @@ process_shortform_attr( >> } >> } >> >> - /* namecheck checks for / and null terminated for file names. >> - * attributes names currently follow the same rules. >> - */ >> + /* namecheck checks for null chars in attr names. */ >> if (namecheck((char *)¤tentry->nameval[0], >> - currententry->namelen)) { >> + currententry->namelen, false)) { > > Hmmmm. that's kinda messy. How about: > > /* attr_namecheck checks for null chars in attr names. */ > bool > attr_namecheck( > uint8_t name, > int length) > { > return namecheck((char *)name, length, false); > } Ok, good idea. -Eric