Thanks Craig.
I think the bugs affect multiple routines:
1.  BackupPC_attribPrint print the mtime of v3 stored attribs as
    unsigned integers so that the 2's complement is displayed for
    negative mtimes (32-bit for v3, 64-bit for v4)
    i.e this bug affects both v3 and v4

2. BackupPC_migrateV3toV4, sets the mtime to 0 for negative mtime
files

3. For V3, BackupPC_tarCreate (and presumably BackupPC_restore) sets the mtime
   of restored files to the unsigned value. Meaning that originally negative
   mtime's are set to the future!

Interestingly, this bug is thus both a v3 and v4 bug.
Surprising perhaps that it was never caught in v3 - i.e. that nobody ever had
a negative mtime file (especially given that both macs and windows
have an epoch before 1/1/1970)

Jeff

Craig Barratt wrote at about 23:30:34 -0700 on Tuesday, April 9, 2019:
 > Jeff,
 > 
 > Thanks for the detective work and proposed fix.  Yes, I assumed mtime was
 > unsigned, when in fact it might not be.
 > 
 > Let me check into the various scenarios of v3/v4 read/write.  Hopefully
 > I'll have time to push fixes this weekend.
 > 
 > Craig
 > 
 > On Mon, Apr 8, 2019 at 7:38 PM <backu...@kosowsky.org> wrote:
 > 
 > > So, the problem seems to be that backuppc treats mtime as an unsigned
 > > integer -- meaning that any file created before Jan 1, 1970 is not
 > > representable.
 > >
 > > v3 represents treats the number as an unsigned integer and basically
 > > shows it's 2's complement (in 32 bits) - i.e. it's packed as an
 > > unsigned 32 bit integer ('N' type in Perl)
 > >
 > > v4 assumes a negative number is an error, sets it to zero, then
 > > defeating the purpose displays it as the 2's complement of 0! v4
 > > stores the number as a variable length unsigned integer.
 > >
 > > According to Wikipedia,
 > >           The Unix time_t data type that represents a point in time is, on
 > > many
 > >           platforms, a signed integer, traditionally of 32 bits (but see
 > > below),
 > >           directly encoding the Unix time number as described in the
 > > preceding
 > >           section. Being 32 bits means that it covers a range of about 136
 > > years
 > >           in total. The minimum representable date is Friday 1901-12-13,
 > > and the
 > >           maximum representable date is Tuesday 2038-01-19. One second
 > > after
 > >           03:14:07 UTC 2038-01-19 this representation will overflow. This
 > >           milestone is anticipated with a mixture of amusement and
 > > dread—see
 > >           year 2038 problem.
 > >
 > >           In some newer operating systems, time_t has been widened to 64
 > >           bits. This expands the times representable by approximately 293
 > >           billion years in both directions, which is over twenty times the
 > >           present age of the universe per direction.
 > >
 > >           There was originally some controversy over whether the Unix
 > > time_t
 > >           should be signed or unsigned. If unsigned, its range in the
 > > future
 > >           would be doubled, postponing the 32-bit overflow (by 68
 > >           years). However, it would then be incapable of representing times
 > >           prior to the epoch. The consensus is for time_t to be signed,
 > > and this
 > >           is the usual practice.
 > >
 > > Seems to me that would be best to continue to represent time as a 32 bit
 > > unsigned
 > > integer in v3 attrib files, represented using 2's complements, so that
 > > any mtime > 2^31 (about 68 years) would be considered as a negative
 > > time -- this seems to be the current implied convention.
 > >
 > > In v4, time would be written as 64 bit unsigned integer using
 > > SetVarInt where any mtime > 2^63 (about 292 billion years from now)
 > > would be considered a negative number written in 2's complements
 > >
 > > Specifically, rather than testing for the sign of 'value' in
 > > setVarInt, it might be better just to cast value to an unsigned 64-bit
 > > integer that is then variable-length encoded as is done currently.
 > >
 > > I am rusty on my 'c' but maybe just:
 > >
 > > static void setVarInt(uchar **bufPP, uchar *bufEnd, int64 value)
 > > {
 > > +    unsigned int64 uvalue = value;
 > >      uchar *bufP = *bufPP;
 > >
 > > -    if ( value < 0 ) {
 > > -        bpc_logErrf("setVarInt botch: got negative argument %ld; setting
 > > to 0\n", (long int)value);
 > > -        value = 0;
 > > -    }
 > >      do {
 > > -        uchar c = value & 0x7f;
 > > +        uchar c = uvalue & 0x7f;
 > >         value >>= 7;
 > > +       uvalue >>= 7;
 > > -        if ( value ) c |= 0x80;
 > > +        if ( uvalue ) c |= 0x80;
 > >         if ( bufP < bufEnd ) {
 > >             *bufP++ = c;
 > >         } else {
 > >             bufP++;
 > >         }
 > > -    } while ( value );
 > > +    } while ( uvalue );
 > >     *bufPP = bufP;
 > > }
 > >
 > >
 > > Both v3 and V4 versions of BackupPC_attribPrint then should
 > > additionally be "fixed" to print mtime as a signed integer converting
 > > from a 32 or 64 bit unsigned integer depending on whether the attrib
 > > file was created with v3 or v4.
 > >
 > > backu...@kosowsky.org wrote at about 21:56:08 -0400 on Monday, April 8,
 > > 2019:
 > >  > I should have said that the v3 case is 2's complement for 32 bit
 > >  > integers while the v4 case is 2's complement for 64 bit integers.
 > >  >
 > >  > Also, the internal representation is clearly right in that both v3 and
 > >  > v4 are retrieving the right number, just displaying it incorrectly.
 > >  >
 > >  > - v3 is showing it as an unsigned rather than signed integer.
 > >  > - v4 is realizing it is an unsigned integer but rejecting it as negative
 > >  >   and then erroneously setting it to zero (and topping on the cake
 > >  >   recording it as the 64 bit 2's complement of zero)
 > >  >
 > >  > backu...@kosowsky.org wrote at about 21:51:27 -0400 on Monday, April
 > > 8, 2019:
 > >  >  > OK, figured it out... VERY INTERESTING.
 > >  >  >
 > >  >  > The original file has an mtime of -2082826800 which corresponds to
 > > 5AM
 > >  >  > Friday, Jan 1, 1904.
 > >  >  > The the error message in question is actually *right* and not an
 > >  >  > error:
 > >  >  >     setVarInt botch: got negative argument -2082826800; setting to 0
 > >  >  >
 > >  >  > i.e. it truly is a negative mtime.
 > >  >  >
 > >  >  > The reason for this odd time is that the file was created on a Mac128
 > >  >  > (circa 1985) and apparently, on old Macs (pre-PowerPC), time starts
 > > with midnight Jan
 > >  >  > 1, 1904 and runs until 2040. Now 5AM GMT is actually 12AM EST (my
 > > home
 > >  >  > timezone), so the file shows a 'default' old Mac start date of
 > >  >  > midnight Jan 1, 1904.
 > >  >  >
 > >  >  > Now for the interesting part.
 > >  >  > Both the v3 and v4 versions of BackupPC_attribPrint are actually
 > > wrong! 2 (different) bugs!!!!
 > >  >  >
 > >  >  > 1. v3 displays a positive mtime of 2212140496 which is is right only
 > >  >  >    in the sense that it is the decimal equivalent of the 2's
 > >  >  >    complement representation of -2082826800
 > >  >  >
 > >  >  > 2. v4 properly catches that the mtime is negative, but signals an
 > >  >  >    error rather than writing it. it then proceeds however to record a
 > >  >  >    wrong (and very large) mtime of 18446744071626724816, rather than
 > >  >  >    0. The number however is 2^64 which is the 2's complement of 0 for
 > >  >  >    8 byte integers
 > >  >  >
 > >  >  >
 > >  >  > Fascinating. I learned a lot from nailing this one down.
 > >  >  > Now both the v3 and v4 bugs should presumably be corrected :)
 > >  >  >
 > >  >  > "" wrote at about 20:23:18 -0400 on Monday, April 8, 2019:
 > >  >  >  > "" wrote at about 20:13:33 -0400 on Monday, April 8, 2019:
 > >  >  >  >
 > >  >  >  > Once again seems to be a problem with bpc_attrib.c:
 > >  >  >  > BackupPC_attribPrint
 > >  >  >  >
 > >  >  >  > Using v3:
 > >  >  >  >  'MacWrite 5.0Scrap' => {
 > >  >  >  >    'uid' => 501,
 > >  >  >  >    'mode' => 33188,
 > >  >  >  >    'sizeDiv4GB' => 0,
 > >  >  >  >    'size' => 0,
 > >  >  >  >    'type' => 0,
 > >  >  >  >    'gid' => 501,
 > >  >  >  >    'sizeMod4GB' => 0,
 > >  >  >  >    'mtime' => 2212140496
 > >  >  >  >   },
 > >  >  >  >
 > >  >  >  > Using v4:
 > >  >  >  >
 > >  >  >  >  'MacWrite 5.0Scrap' => {
 > >  >  >  >    'compress' => 3,
 > >  >  >  >    'digest' => '',
 > >  >  >  >    'gid' => 501,
 > >  >  >  >    'inode' => 0,
 > >  >  >  >    'mode' => 33188,
 > >  >  >  >    'mtime' => '18446744071626724816',
 > >  >  >  >    'name' => 'MacWrite 5.0Scrap',
 > >  >  >  >    'nlinks' => 0,
 > >  >  >  >    'size' => 0,
 > >  >  >  >    'type' => 0,
 > >  >  >  >    'uid' => 501
 > >  >  >  >   },
 > >  >  >  >
 > >  >  >  > Clearly, v4 is messing up on reading 'mtime' which presumably is
 > > what
 > >  >  >  > is causing the problem, consistent with the verbose output of
 > >  >  >  > BackupPC_migrateV3toV4 (as more fully excertped below)
 > >  >  >  >
 > >  >  >  >  > setVarInt botch: got negative argument -2082826800; setting to
 > > 0
 > >  >  >  >  > Wrote file MacWrite 5.0Scrap: type = 0, mode = 0100644,
 > > uid/gid = 501/501, size = 0, inode = 11616435, nlinks = 0, digest =
 > > 0xd41d8c..., bufUsed = 925
 > >  >  >  >
 > >  >  >  >
 > >  >  >  >
 > >  >  >  >  >
 > >  >  >  >  > Craig Barratt wrote at about 23:39:50 -0700 on Sunday, April
 > > 7, 2019:
 > >  >  >  >  >  > Jeff,
 > >  >  >  >  >  >
 > >  >  >  >  >  > Please send me an attrib file that triggers that error.
 > >  >  >  >  >  >
 > >  >  >  >  >  > Craig
 > >  >  >  >  >  >
 > >  >  >  >  >
 > >  >  >  >  > Here is the relevant verbose output from BackupPc_migrateV3toV4
 > >  >  >  >  > starting with the attrib file:
 > >  >  >  >  >
 > >  >  >  >  > bpc_poolRefDeltaUpdate(5a3631521955e47e179ea20d8a84ac5a, 1),
 > > count now 1
 > >  >  >  >  >
 > > bpc_attrib_dirWrite(/var/lib/backuppc/pc/consult/13.v4/./f%2f/fvar/fopt/fexecutor/fshare/fhome/fSystem
 > > Folder/attrib): dirPath =
 > > /var/lib/backuppc/pc/consult/13.v4/./f%2f/fvar/fopt/fexecutor/fshare/fhome/fSystem
 > > Folder, attribFileName = attrib, baseAttribFileName = attrib
 > >  >  >  >  >
 > > bpc_path_create(/var/lib/backuppc/pc/consult/13.v4/./f%2f/fvar/fopt/fexecutor/fshare/fhome/fSystem
 > > Folder)
 > >  >  >  >  > Wrote file Extensions: type = 5, mode = 040771, uid/gid =
 > > 0/1002, size = 4096, inode = 11616442, nlinks = 0, digest = 0x000000...,
 > > bufUsed = 4
 > >  >  >  >  > Wrote file %mac.rsrc: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 6753, inode = 11616437, nlinks = 0, digest = 0xc874b4...,
 > > bufUsed = 37
 > >  >  >  >  > Wrote file system.ard: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 0, inode = 11616455, nlinks = 0, digest = 0xd41d8c...,
 > > bufUsed = 85
 > >  >  >  >  > Wrote file %MacWrite 5.0Scrap: type = 0, mode = 0100644,
 > > uid/gid = 501/501, size = 198, inode = 11616432, nlinks = 0, digest =
 > > 0x85e52a..., bufUsed = 133
 > >  >  >  >  > Wrote file Excel Toolbars: type = 0, mode = 0100644, uid/gid =
 > > 501/501, size = 267, inode = 11616447, nlinks = 0, digest = 0x619077...,
 > > bufUsed = 191
 > >  >  >  >  > Wrote file %ParamRAM: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 198, inode = 11616457, nlinks = 0, digest = 0x5a3631...,
 > > bufUsed = 245
 > >  >  >  >  > Wrote file Printer: type = 0, mode = 0100640, uid/gid =
 > > 0/1002, size = 0, inode = 11616441, nlinks = 0, digest = 0xd41d8c...,
 > > bufUsed = 293
 > >  >  >  >  > Wrote file %system.ard: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 798, inode = 11616445, nlinks = 0, digest = 0xdcd6d0...,
 > > bufUsed = 338
 > >  >  >  >  > Wrote file %System: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 487415, inode = 11616434, nlinks = 0, digest = 0xa9c2a3...,
 > > bufUsed = 388
 > >  >  >  >  > Wrote file System: type = 0, mode = 0100660, uid/gid = 0/1002,
 > > size = 0, inode = 11616433, nlinks = 0, digest = 0xd41d8c..., bufUsed = 435
 > >  >  >  >  > Wrote file Browser: type = 0, mode = 0100640, uid/gid =
 > > 0/1002, size = 0, inode = 11616450, nlinks = 0, digest = 0xd41d8c...,
 > > bufUsed = 479
 > >  >  >  >  > Wrote file ParamRAM: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 20, inode = 11616443, nlinks = 0, digest = 0xcef2a5...,
 > > bufUsed = 524
 > >  >  >  >  > Wrote file %Printer: type = 0, mode = 0100640, uid/gid =
 > > 0/1002, size = 2279, inode = 11616438, nlinks = 0, digest = 0x07c53e...,
 > > bufUsed = 570
 > >  >  >  >  > Wrote file %Browser: type = 0, mode = 0100640, uid/gid =
 > > 0/1002, size = 119268, inode = 11616448, nlinks = 0, digest = 0x9fb0e6...,
 > > bufUsed = 617
 > >  >  >  >  > Wrote file %windows.rsrc: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 15917, inode = 11616451, nlinks = 0, digest = 0xd30945...,
 > > bufUsed = 665
 > >  >  >  >  > Wrote file %Excel Toolbars: type = 0, mode = 0100644, uid/gid
 > > = 501/501, size = 198, inode = 11616439, nlinks = 0, digest = 0x7c1511...,
 > > bufUsed = 717
 > >  >  >  >  > Wrote file windows.rsrc: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 0, inode = 11616453, nlinks = 0, digest = 0xd41d8c...,
 > > bufUsed = 772
 > >  >  >  >  > Wrote file mac.rsrc: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 0, inode = 11616440, nlinks = 0, digest = 0xd41d8c...,
 > > bufUsed = 822
 > >  >  >  >  > Wrote file Excel Settings (4): type = 0, mode = 0100644,
 > > uid/gid = 501/501, size = 0, inode = 11616436, nlinks = 0, digest =
 > > 0xd41d8c..., bufUsed = 868
 > >  >  >  >  > setVarInt botch: got negative argument -2082826800; setting to
 > > 0
 > >  >  >  >  > Wrote file MacWrite 5.0Scrap: type = 0, mode = 0100644,
 > > uid/gid = 501/501, size = 0, inode = 11616435, nlinks = 0, digest =
 > > 0xd41d8c..., bufUsed = 925
 > >  >  >  >  > Wrote file %godata.sav: type = 0, mode = 0100644, uid/gid =
 > > 501/501, size = 198, inode = 11616452, nlinks = 0, digest = 0x39c291...,
 > > bufUsed = 977
 > >  >  >  >  > Wrote file %Preferences: type = 0, mode = 0100660, uid/gid =
 > > 0/1002, size = 198, inode = 11616454, nlinks = 0, digest = 0x087760...,
 > > bufUsed = 1028
 > >  >  >  >  > Wrote file godata.sav: type = 0, mode = 0100644, uid/gid =
 > > 501/501, size = 1100, inode = 11616456, nlinks = 0, digest = 0xb247c3...,
 > > bufUsed = 1079
 > >  >  >  >  > Wrote file MacWrite 5.0Undo: type = 0, mode = 0100644, uid/gid
 > > = 501/501, size = 32, inode = 11616446, nlinks = 0, digest = 0x70bc8f...,
 > > bufUsed = 1129
 > >  >  >  >  > Wrote file %MacWrite 5.0Undo: type = 0, mode = 0100644,
 > > uid/gid = 501/501, size = 198, inode = 11616449, nlinks = 0, digest =
 > > 0xb5c763..., bufUsed = 1184
 > >  >  >  >  > Wrote file Preferences: type = 5, mode = 040771, uid/gid =
 > > 0/1002, size = 4096, inode = 11616431, nlinks = 0, digest = 0x000000...,
 > > bufUsed = 1241
 > >  >  >  >  >
 > >  >  >  >  >
 > >  >  >  >  > I am also attaching the relevant attrib file
 > >  >  >  >  > [DELETED ATTACHMENT attrib, Untyped binary data]
 > >  >  >
 > >  >  >
 > >  >  > _______________________________________________
 > >  >  > BackupPC-users mailing list
 > >  >  > BackupPC-users@lists.sourceforge.net
 > >  >  > List:    https://lists.sourceforge.net/lists/listinfo/backuppc-users
 > >  >  > Wiki:    http://backuppc.wiki.sourceforge.net
 > >  >  > Project: http://backuppc.sourceforge.net/
 > >  >
 > >  >
 > >  > _______________________________________________
 > >  > BackupPC-users mailing list
 > >  > BackupPC-users@lists.sourceforge.net
 > >  > List:    https://lists.sourceforge.net/lists/listinfo/backuppc-users
 > >  > Wiki:    http://backuppc.wiki.sourceforge.net
 > >  > Project: http://backuppc.sourceforge.net/
 > >


_______________________________________________
BackupPC-users mailing list
BackupPC-users@lists.sourceforge.net
List:    https://lists.sourceforge.net/lists/listinfo/backuppc-users
Wiki:    http://backuppc.wiki.sourceforge.net
Project: http://backuppc.sourceforge.net/

Reply via email to