Date: Tue, 5 Dec 2006 00:10:19 GMT From: Bruce Evans <bde@zeta.org.au> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/106255: [msdosfs] : correct setting of archive flag Message-ID: <200612050010.kB50AJZD032972@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/106255; it has been noted by GNATS. From: Bruce Evans <bde@zeta.org.au> To: Rene Ladan <r.c.ladan@gmail.com> Cc: freebsd-gnats-submit@FreeBSD.org, freebsd-bugs@FreeBSD.org Subject: Re: kern/106255: [msdosfs] : correct setting of archive flag Date: Tue, 5 Dec 2006 11:07:36 +1100 (EST) On Mon, 4 Dec 2006, Rene Ladan wrote: > Bruce Evans schreef: >> On Sun, 3 Dec 2006, Rene Ladan wrote: >> >>>> Description: >>> The MSDOS file system has an archive bit in the flags field. This bit >>> roughly corresponds to the archive flag on the UFS file system. >>> However, it is set the wrong way around: the flag should be set when >>> the bit is present, and cleared when the bit is absent. >> >> The comment in msdosfs/direntry.h says that ATTR_ARCHIVE means that >> the file is new or modified (in other words, not archived), while the >> comment in sys/stat.h says that SF_ARCHIVED means that the file is >> archived, but I think both mean that it is archived. >> > Indeed, the MSDOS archive bit means that the user should archive the file. Not indeed. The MSDOS archive bit does mean that the file needs to be archived, but this means that it has the opposite sense to SF_ARCHIVED and the main bug is in the patch in the PR. >>> --- msdosfs_vnops.c Mon Nov 6 14:41:57 2006 >>> +++ msdosfs_vnops.c.rene Sun Dec 3 11:58:47 2006 >>> @@ -352,7 +352,7 @@ >>> vap->va_ctime = vap->va_mtime; >>> } >>> vap->va_flags = 0; >>> - if ((dep->de_Attributes & ATTR_ARCHIVE) == 0) >>> + if (dep->de_Attributes & ATTR_ARCHIVE) >>> vap->va_flags |= SF_ARCHIVED; >>> vap->va_gen = 0; >>> vap->va_blocksize = pmp->pm_bpcluster; >> >> This only fixes the reporting of the flag. msdosfs still maintains >> the flag perfectly backwards (except DETIMES() is missing setting of >> it for for all changes -- I think all changes to metadata except >> possibly to atimes should set it to be perfectly backwards and clear >> it to be correct). >> > Thanks, I'll look into that. The above actually only breaks the reporting of the flag. ATTR_ARCHIVE has the opposite sese to SF_ARCHIVED, so !ATTR_ARCHIVED must be converted to SF_ARCHIVED as in the original code above, and the reverse conversion must be done when setting msdosfs attributes from FreeBSD attributes (as done in msdosfs_setattr()). At the user level, thus means that when ls -o displays "attr", it means that the file is archived, but when an msdosfs's attrib utility displays "A" it means that the file is not archived. The main bug here seems to be just for directories. The archive attribute doesn't apply for directories, and the above doesn't have a special case for directories, so the above tests garbage for the directory case. The garbage is usually (maybe always?) 0, so SF_ARVCHIVED is usually set for directories and ls -o displays "attr". This is consistent with attrib not displaying "A" but strange. msdosfs_setattr() is more careful with the archive bit for directories, but still wrong. It silently ignores attempts to set this bit. Thus applications like cp -p succeed where they should fail, masking the bug in msdosfs_getattr(): msdosfs_getattr() bogusly turns the absence of an ATTR_ARCHIVE bit into a setting of SF_ARCHIVED; cp -p tries to preserve this setting but cannot, and the error goes undetected since msdosfs_setattr() silently ignores it. More in another reply. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200612050010.kB50AJZD032972>