Is the handling for bug#2045 enough? (MS-Office behavior of
timestamp)
Jeremy Allison
jra at samba.org
Thu Mar 10 01:26:52 GMT 2005
On Wed, Mar 09, 2005 at 07:18:52PM +0900, ke_miyata at itg.hitachi.co.jp wrote:
> I add some technical background to my prior posting and attach a
> patch to the problem.
>
> I think the change made to the Bug #2045 has two problems.
> (1) It can't handle PowerPoint 2000 behavior of changing
> timestamps.
> (2) A process for TRANS2/SETPATHINFO is degraded.
Ok, here is my modified patch (applies against SAMBA_3_0 svn).
Let me know if it fixes your issues (I'm still not comfortable
with the close issue - I need further explaination).
Jeremy.
-------------- next part --------------
Index: smbd/nttrans.c
===================================================================
--- smbd/nttrans.c (revision 5717)
+++ smbd/nttrans.c (working copy)
@@ -1698,7 +1698,7 @@
close_file(fsp1,False);
/* Ensure the modtime is set correctly on the destination file. */
- fsp2->pending_modtime = sbuf1.st_mtime;
+ fsp_set_pending_modtime(fsp2, sbuf1.st_mtime);
close_ret = close_file(fsp2,False);
Index: smbd/reply.c
===================================================================
--- smbd/reply.c (revision 5717)
+++ smbd/reply.c (working copy)
@@ -2876,6 +2876,13 @@
conn->num_files_open));
/*
+ * Take care of any time sent in the close.
+ */
+
+ mtime = make_unix_date3(inbuf+smb_vwv1);
+ fsp_set_pending_modtime(fsp, mtime);
+
+ /*
* close_file() returns the unix errno if an error
* was detected on close - normally this is due to
* a disk full error. If not then it was probably an I/O error.
@@ -2886,16 +2893,6 @@
END_PROFILE(SMBclose);
return (UNIXERROR(ERRHRD,ERRgeneral));
}
-
- /*
- * Now take care of any time sent in the close.
- */
-
- mtime = make_unix_date3(inbuf+smb_vwv1);
-
- /* try and set the date */
- set_filetime(conn, file_name, mtime);
-
}
/* We have a cached error */
@@ -4233,7 +4230,7 @@
close_file(fsp1,False);
/* Ensure the modtime is set correctly on the destination file. */
- fsp2->pending_modtime = src_sbuf.st_mtime;
+ fsp_set_pending_modtime( fsp2, src_sbuf.st_mtime);
/*
* As we are opening fsp1 read-only we only expect
@@ -4917,7 +4914,7 @@
* Sometimes times are sent as zero - ignore them.
*/
- if ((unix_times.actime == 0) && (unix_times.modtime == 0)) {
+ if (null_mtime(unix_times.actime) && null_mtime(unix_times.modtime)) {
/* Ignore request */
if( DEBUGLVL( 3 ) ) {
dbgtext( "reply_setattrE fnum=%d ", fsp->fnum);
@@ -4925,12 +4922,13 @@
}
END_PROFILE(SMBsetattrE);
return(outsize);
- } else if ((unix_times.actime != 0) && (unix_times.modtime == 0)) {
- /* set modify time = to access time if modify time was 0 */
+ } else if (!null_mtime(unix_times.actime) && null_mtime(unix_times.modtime)) {
+ /* set modify time = to access time if modify time was unset */
unix_times.modtime = unix_times.actime;
}
/* Set the date on this file */
+ /* Should we set pending modtime here ? JRA */
if(file_utime(conn, fsp->fsp_name, &unix_times)) {
END_PROFILE(SMBsetattrE);
return ERROR_DOS(ERRDOS,ERRnoaccess);
@@ -5170,6 +5168,7 @@
put_dos_date2(outbuf,smb_vwv0,get_create_time(&sbuf,lp_fake_dir_create_times(SNUM(conn))));
put_dos_date2(outbuf,smb_vwv2,sbuf.st_atime);
+ /* Should we check pending modtime here ? JRA */
put_dos_date2(outbuf,smb_vwv4,sbuf.st_mtime);
if (mode & aDIR) {
Index: smbd/files.c
===================================================================
--- smbd/files.c (revision 5717)
+++ smbd/files.c (working copy)
@@ -37,6 +37,13 @@
static int files_used;
+/* A singleton cache to speed up searching by dev/inode. */
+static struct fsp_singleton_cache {
+ files_struct *fsp;
+ SMB_DEV_T dev;
+ SMB_INO_T inode;
+} fsp_fi_cache;
+
/****************************************************************************
Return a unique number identifying this fsp over the life of this pid.
****************************************************************************/
@@ -122,6 +129,11 @@
i, fsp->fnum, files_used));
chain_fsp = fsp;
+
+ /* A new fsp invalidates a negative fsp_fi_cache. */
+ if (fsp_fi_cache.fsp == NULL) {
+ ZERO_STRUCT(fsp_fi_cache);
+ }
return fsp;
}
@@ -299,19 +311,34 @@
/****************************************************************************
Find the first fsp given a device and inode.
+ We use a singleton cache here to speed up searching from getfilepathinfo
+ calls.
****************************************************************************/
files_struct *file_find_di_first(SMB_DEV_T dev, SMB_INO_T inode)
{
files_struct *fsp;
+ if (fsp_fi_cache.dev == dev && fsp_fi_cache.inode == inode) {
+ /* Positive or negative cache hit. */
+ return fsp_fi_cache.fsp;
+ }
+
+ fsp_fi_cache.dev = dev;
+ fsp_fi_cache.inode = inode;
+
for (fsp=Files;fsp;fsp=fsp->next) {
if ( fsp->fd != -1 &&
fsp->dev == dev &&
- fsp->inode == inode )
+ fsp->inode == inode ) {
+ /* Setup positive cache. */
+ fsp_fi_cache.fsp = fsp;
return fsp;
+ }
}
+ /* Setup negative cache. */
+ fsp_fi_cache.fsp = NULL;
return NULL;
}
@@ -342,13 +369,36 @@
files_struct *fsp;
for (fsp=Files;fsp;fsp=fsp->next) {
- if (fsp->print_file) return fsp;
+ if (fsp->print_file) {
+ return fsp;
+ }
}
return NULL;
}
/****************************************************************************
+ Set a pending modtime across all files with a given dev/ino pair.
+****************************************************************************/
+
+void fsp_set_pending_modtime(files_struct *tfsp, time_t pmod)
+{
+ files_struct *fsp;
+
+ if (null_mtime(pmod)) {
+ return;
+ }
+
+ for (fsp = Files;fsp;fsp=fsp->next) {
+ if ( fsp->fd != -1 &&
+ fsp->dev == tfsp->dev &&
+ fsp->inode == tfsp->inode ) {
+ fsp->pending_modtime = pmod;
+ }
+ }
+}
+
+/****************************************************************************
Sync open files on a connection.
****************************************************************************/
@@ -392,6 +442,11 @@
chain_fsp = NULL;
}
+ /* Closing a file can invalidate the positive cache. */
+ if (fsp == fsp_fi_cache.fsp) {
+ ZERO_STRUCT(fsp_fi_cache);
+ }
+
SAFE_FREE(fsp);
}
Index: smbd/trans2.c
===================================================================
--- smbd/trans2.c (revision 5717)
+++ smbd/trans2.c (working copy)
@@ -2469,9 +2469,18 @@
c_time = get_create_time(&sbuf,lp_fake_dir_create_times(SNUM(conn)));
- if (fsp && fsp->pending_modtime) {
- /* the pending modtime overrides the current modtime */
- sbuf.st_mtime = fsp->pending_modtime;
+ if (fsp) {
+ if (fsp->pending_modtime) {
+ /* the pending modtime overrides the current modtime */
+ sbuf.st_mtime = fsp->pending_modtime;
+ }
+ } else {
+ /* Do we have this path open ? */
+ files_struct *fsp1 = file_find_di_first(sbuf.st_dev, sbuf.st_ino);
+ if (fsp1 && fsp1->pending_modtime) {
+ /* the pending modtime overrides the current modtime */
+ sbuf.st_mtime = fsp1->pending_modtime;
+ }
}
if (lp_dos_filetime_resolution(SNUM(conn))) {
@@ -3323,9 +3332,9 @@
tvs.modtime = write_time;
}
/* Prefer a defined time to an undefined one. */
- if (tvs.modtime == (time_t)0 || tvs.modtime == (time_t)-1)
- tvs.modtime = (write_time == (time_t)0 || write_time == (time_t)-1
- ? changed_time : write_time);
+ if (null_mtime(tvs.modtime)) {
+ tvs.modtime = null_mtime(write_time) ? changed_time : write_time;
+ }
/* attributes */
dosmode = IVAL(pdata,32);
@@ -3805,11 +3814,13 @@
}
/* get some defaults (no modifications) if any info is zero or -1. */
- if (tvs.actime == (time_t)0 || tvs.actime == (time_t)-1)
+ if (null_mtime(tvs.actime)) {
tvs.actime = sbuf.st_atime;
+ }
- if (tvs.modtime == (time_t)0 || tvs.modtime == (time_t)-1)
+ if (null_mtime(tvs.modtime)) {
tvs.modtime = sbuf.st_mtime;
+ }
DEBUG(6,("actime: %s " , ctime(&tvs.actime)));
DEBUG(6,("modtime: %s ", ctime(&tvs.modtime)));
@@ -3841,31 +3852,7 @@
* Try and set the times, size and mode of this file -
* if they are different from the current values
*/
- if (sbuf.st_mtime != tvs.modtime || sbuf.st_atime != tvs.actime) {
- if(fsp != NULL) {
- /*
- * This was a setfileinfo on an open file.
- * NT does this a lot. We also need to
- * set the time here, as it can be read by
- * FindFirst/FindNext and with the patch for bug #2045
- * in smbd/fileio.c it ensures that this timestamp is
- * kept sticky even after a write. We save the request
- * away and will set it on file close and after a write. JRA.
- */
- if (tvs.modtime != (time_t)0 && tvs.modtime != (time_t)-1) {
- DEBUG(10,("call_trans2setfilepathinfo: setting pending modtime to %s\n", ctime(&tvs.modtime) ));
- fsp->pending_modtime = tvs.modtime;
- }
-
- DEBUG(10,("call_trans2setfilepathinfo: setting utimes to modified values.\n"));
-
- if(file_utime(conn, fname, &tvs)!=0) {
- return(UNIXERROR(ERRDOS,ERRnoaccess));
- }
- }
- }
-
/* check the mode isn't different, before changing it */
if ((dosmode != 0) && (dosmode != dos_mode(conn, fname, &sbuf))) {
@@ -3877,6 +3864,7 @@
}
}
+ /* Now the size. */
if (size != get_file_size(sbuf)) {
int ret;
@@ -3917,6 +3905,34 @@
return (UNIXERROR(ERRHRD,ERRdiskfull));
}
+ /*
+ * Finally the times.
+ */
+ if (sbuf.st_mtime != tvs.modtime || sbuf.st_atime != tvs.actime) {
+ if(fsp != NULL) {
+ /*
+ * This was a setfileinfo on an open file.
+ * NT does this a lot. We also need to
+ * set the time here, as it can be read by
+ * FindFirst/FindNext and with the patch for bug #2045
+ * in smbd/fileio.c it ensures that this timestamp is
+ * kept sticky even after a write. We save the request
+ * away and will set it on file close and after a write. JRA.
+ */
+
+ if (tvs.modtime != (time_t)0 && tvs.modtime != (time_t)-1) {
+ DEBUG(10,("call_trans2setfilepathinfo: setting pending modtime to %s\n", ctime(&tvs.modtime) ));
+ fsp_set_pending_modtime(fsp, tvs.modtime);
+ }
+
+ }
+ DEBUG(10,("call_trans2setfilepathinfo: setting utimes to modified values.\n"));
+
+ if(file_utime(conn, fname, &tvs)!=0) {
+ return(UNIXERROR(ERRDOS,ERRnoaccess));
+ }
+ }
+
SSVAL(params,0,0);
send_trans2_replies(outbuf, bufsize, params, 2, *ppdata, 0);
More information about the samba-technical
mailing list