bugConcurrent Versions System - Bugs: bug #22710, cvs 1.11.22 ignoring symlink...

 
 

bug #22710: cvs 1.11.22 ignoring symlink modification time - appears unintentional

Submitted by:  Nelson B Bolyard <nelsonb>
Submitted on:  Mon 24 Mar 2008 12:37:47 AM UTC  
 
Category: Bug Fix (patch attached)Severity: 3 - Normal
Item Group: NoneStatus: Fixed
Privacy: PublicAssigned to: Mark D. Baushke <mdb>
Open/Closed: ClosedRelease: 
Fixed Release: 1.11.23Fixed Feature Release: None

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon 24 Mar 2008 09:02:23 AM UTC, comment #1:

Your code fix is the correct fix. This problem does not
exist in the FEATURE branch (fixed in cvs-1.12.9).

Mark D. Baushke <mdb>
Project AdministratorIn charge of this item.
Mon 24 Mar 2008 12:37:47 AM UTC, original submission:

In the cvs 1.11.22 sources, file src/vers_ts.c, function time_stamp(), there is a block comment that appears to document how the function is intended to work. It says:

/* If it's a symlink, return whichever is the newest mtime of
the link and its target, for safety.
*/

And there is code that does both an lstat and a stat, and computes the lesser of the two modification times, e.g.

struct stat sb;
time_t mtime = 0L;
if (!CVS_LSTAT (file, &sb))
{
mtime = sb.st_mtime;
}
if (!CVS_STAT (file, &sb))
{
if (mtime < sb.st_mtime)
mtime = sb.st_mtime;
}

But the value of this computed mtime is not used in the
subsequent computations. Instead, the value of sb.st_mtime
is subsequently used.

tm_p = gmtime (&sb.st_mtime);
cp = tm_p ? asctime (tm_p) : ctime (&sb.st_mtime);

This means that the comparison of the two mtimes is not used.
The actual behavior is to use the value left in the stat buf
after stat() returns. Most of the time, this will be the
value returned by stat. The only way that sb.st_mtime can be
the value from lstat is if lstat succeeds but stat fails.

This tiny patch makes the code work the way that the comment
says it was intended to work.

- tm_p = gmtime (&sb.st_mtime);
- cp = tm_p ? asctime (tm_p) : ctime (&sb.st_mtime);
+ tm_p = gmtime (&mtime);
+ cp = tm_p ? asctime (tm_p) : ctime (&mtime);

However, it's not entirely clear to me which is the more
desirable behavior: the way the code actually works now,
or the way that the comments say it was intended to work.

Nelson B Bolyard <nelsonb>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by mdb (Posted a comment)
  • -unavailable- added by nelsonb (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 24 Mar 2008 09:02:23 AM UTCmdbStatusNone=>Fixed
      Assigned toNone=>mdb
      Open/ClosedOpen=>Closed
      Fixed ReleaseNone=>1.11.23

    Back to the top


    Powered by Savane 3.1-cleanup1