kern/106255: [msdosfs] : correct setting of archive flag

[Rene Ladan]

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.
> 
[patch 1]

> 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).
>
I've attached a new patch which
  * fixes reporting of the flag (as in the previous patch)
  * sets the archive flag in DETIMES() when the file is created
  * cleans up the logic of not supporting setting the archive flag on
directories (chunk 3)
  * does not set the flag when (vap->va_atime.tv_sec != VNOVAL) or
(vap->va_mode != VNOVAL) in msdosfs_setattr()

I think that only userland tools should send a 'clear request', as the
flag only needs to be cleared when the file is backed up.  The kernel
cannot know when a file has been backed up.

Any comments are welcome.

> Bruce
> 

Regards,
Rene
-- 
GPG fingerprint = E738 5471 D185 7013 0EE0  4FC8 3C1D 6F83 12E1 84F6
(subkeys.pgp.net)

"It won't fit on the line."
		-- me, 2001

-------------- next part --------------
--- msdosfs_vnops.c.orig	Sun Dec  3 20:45:24 2006
+++ msdosfs_vnops.c	Mon Dec  4 12:43:37 2006
@@ -354,7 +354,7 @@
 		vap->va_birthtime.tv_nsec = 0;
 	}
 	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;
@@ -431,12 +431,13 @@
 			if (error)
 				return (error);
 		}
-		if (vap->va_flags & ~SF_ARCHIVED)
-			return EOPNOTSUPP;
 		if (vap->va_flags & SF_ARCHIVED)
 			dep->de_Attributes &= ~ATTR_ARCHIVE;
-		else if (!(dep->de_Attributes & ATTR_DIRECTORY))
-			dep->de_Attributes |= ATTR_ARCHIVE;
+		else
+			if (dep->de_Attributes & ATTR_DIRECTORY)
+				return EOPNOTSUPP;
+			else 
+				dep->de_Attributes |= ATTR_ARCHIVE;
 		dep->de_flag |= DE_MODIFIED;
 	}
 
@@ -506,8 +507,9 @@
 				dep->de_flag &= ~DE_UPDATE;
 				timespec2fattime(&vap->va_mtime, 0,
 				    &dep->de_MDate, &dep->de_MTime, NULL);
+				dep->de_Attributes |= ATTR_ARCHIVE;
+				/* only set archive flag when file has changed */
 			}
-			dep->de_Attributes |= ATTR_ARCHIVE;
 			dep->de_flag |= DE_MODIFIED;
 		}
 	}
@@ -531,7 +533,6 @@
 				dep->de_Attributes &= ~ATTR_READONLY;
 			else
 				dep->de_Attributes |= ATTR_READONLY;
-			dep->de_Attributes |= ATTR_ARCHIVE;
 			dep->de_flag |= DE_MODIFIED;
 		}
 	}
--- denode.h.orig	Thu Oct 26 11:21:07 2006
+++ denode.h	Mon Dec  4 12:35:00 2006
@@ -239,6 +239,7 @@
 		timespec2fattime((cre), 0, &(dep)->de_CDate,		\
 		    &(dep)->de_CTime, &(dep)->de_CHun);			\
 		    (dep)->de_flag |= DE_MODIFIED;			\
+		    (dep)->de_Attributes |= ATTR_ARCHIVE;		\
 	}								\
 	(dep)->de_flag &= ~(DE_UPDATE | DE_CREATE | DE_ACCESS);		\
 } while (0)