Wireshark · Wireshark-dev: [Wireshark-dev] FILETIME to nstime_t conversions gives a warning

[Peter Wu <peter@xxxxxxxxxxxxx>]

Hi,

I am getting these compile warnings using Clang 3.7.0:

    wsutil/nstime.c:235:25: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
        time_t l_time_min = TIME_T_MIN;
                            ^~~~~~~~~~
    wsutil/nstime.c:223:36: note: expanded from macro 'TIME_T_MIN'
                        : ~ (time_t) 0 << (sizeof (time_t) * CHAR_BIT - 1)))
                          ~~~~~~~~~~~~ ^
    wsutil/nstime.c:236:25: warning: shifting a negative signed value is undefined [-Wshift-negative-value]
        time_t l_time_max = TIME_T_MAX;
                            ^~~~~~~~~~
    wsutil/nstime.c:226:46: note: expanded from macro 'TIME_T_MAX'
    #define TIME_T_MAX ((time_t) (~ (time_t) 0 - TIME_T_MIN))
                                                 ^~~~~~~~~~
    wsutil/nstime.c:223:36: note: expanded from macro 'TIME_T_MIN'
                        : ~ (time_t) 0 << (sizeof (time_t) * CHAR_BIT - 1)))
                          ~~~~~~~~~~~~ ^

Looking a bit further in the code, I became a bit confused on what is
going on in nstime.c. There is a magic "fixup" constant and some special
handling for the largest and smallest time_t values.

The comments suggest that it is based on Samba code, but their latest
version[1] is much simpler:

    #ifndef TIME_T_MIN
    /* we use 0 here, because (time_t)-1 means error */
    #define TIME_T_MIN 0
    #endif

    /*
     * we use the INT32_MAX here as on 64 bit systems,
     * gmtime() fails with INT64_MAX
     */
    #ifndef TIME_T_MAX
    #define TIME_T_MAX MIN(INT32_MAX,_TYPE_MAXIMUM(time_t))
    #endif

    /* 64 bit time (100 nanosec) 1601 - cifs6.txt, section 3.5, page 30, 4 byte aligned */
    typedef uint64_t NTTIME;

and the conversion method[2]:

    /**
    put a 8 byte filetime from a time_t
    This takes GMT as input
    **/
    _PUBLIC_ void unix_to_nt_time(NTTIME *nt, time_t t)
    {
        uint64_t t2;

        if (t == (time_t)-1) {
            *nt = (NTTIME)-1LL;
            return;
        }

        if (t == TIME_T_MAX || t == INT64_MAX) {
            *nt = 0x7fffffffffffffffLL;
            return;
        }

        if (t == 0) {
            *nt = 0;
            return;
        }

        t2 = t;
        t2 += TIME_FIXUP_CONSTANT_INT;
        t2 *= 1000*1000*10;

        *nt = t2;
    }

Shouldn't we do the same then? (Guy?)
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl

 [1]: https://github.com/samba-team/samba/blob/master/lib/util/time.h#L31
 [2]: https://github.com/samba-team/samba/blob/master/lib/util/time.c#L136