Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 21 Jun 2008 08:54:04 -0700
From:      "Garrett Cooper" <yanefbsd@gmail.com>
To:        "Bruce Evans" <brde@optusnet.com.au>
Cc:        freebsd-bugs@freebsd.org
Subject:   Re: kern/68081: [headers] [patch] sys/time.h (lint fix)
Message-ID:  <7d6fde3d0806210854y1c3be9ds1e1836b1c08ee671@mail.gmail.com>
In-Reply-To: <20080621224835.L67195@delplex.bde.org>
References:  <200806210030.m5L0U3Jr079362@freefall.freebsd.org> <20080621224835.L67195@delplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jun 21, 2008 at 6:22 AM, Bruce Evans <brde@optusnet.com.au> wrote:
> On Sat, 21 Jun 2008, Garrett Cooper wrote:
>
>> Haven't used lint(1) before, but I don't think this is an issue (at
>> least not on 8-CURRENT):
>
> All these bugs (in lint, not in the source code) seem to be unchanged
> in -current.  You probably tested on amd64, where the proposed fix has
> no effect except to add even more style bugs than on i386.
>
> tv_nsec has type long on all machines, and lint doesn't like assigning
> uint64_t's to longs when uint64_t is longer than long.  On amd64,
> uint64_t is u_long so the assignment doesn't reduce the size, and lint
> is apparently too stupid to warn about the sign mismatch.  On i386,
> uint64_t is u_long_long, so the assignment reduces the size.
>
> lint is too stupid to understand that right-shifting a 64-bit type
> makes it fit in a 32-bit type, except possibly for the sign.  A slightly
> more detailed static analysis shows that the value is non-negative,
> (in fact that it is between 0 and 10^9-1 inclusive, as is required for
> a valid tv_nsec), so there is no problem with the sign.  These stupidities
> are bugs in lint, since casting down produces more than enough
> non-spurious warnings.  gcc is smarter, but similar warnings in gcc are
> normally not enabled.
>
> The proposed fix adds the following style bugs:
> all:
> - long lines
> - explicitly cast the type down, but to a wrong type (uint32_t) instead of
>  long, so the full set of conversions is uint64_t -> uint32_t -> long.
>  This is a bit harder to understand.  If lint remains stupid but warns
>  about sign mismatches too, then this should suppress the warning
>  about the size reduction but not a new one about the sign mismatch.
> amd64 and some others:
> - the full set of conversions now gives a promotion from uint32_t to long.
>
>> [gcooper@optimus /devel/ncvs/src]$ lint time_test.c
>> time_test.c:
>> _types.h(60): warning: struct __timer never defined [233]
>> _types.h(61): warning: struct __mq never defined [233]
>> _pthreadtypes.h(44): warning: struct pthread never defined [233]
>> _pthreadtypes.h(45): warning: struct pthread_attr never defined [233]
>> _pthreadtypes.h(46): warning: struct pthread_cond never defined [233]
>> _pthreadtypes.h(47): warning: struct pthread_cond_attr never defined [233]
>> _pthreadtypes.h(48): warning: struct pthread_mutex never defined [233]
>> _pthreadtypes.h(49): warning: struct pthread_mutex_attr never defined
>> [233]
>> _pthreadtypes.h(51): warning: struct pthread_rwlock never defined [233]
>> _pthreadtypes.h(52): warning: struct pthread_rwlockattr never defined
>> [233]
>> _pthreadtypes.h(53): warning: struct pthread_barrier never defined [233]
>> _pthreadtypes.h(54): warning: struct pthread_barrier_attr never defined
>> [233]
>> _pthreadtypes.h(55): warning: struct pthread_spinlock never defined [233]
>> _pthreadtypes.h(75): warning: struct pthread_barrierattr never defined
>> [233]
>> time.h(59): warning: static function bintime_addx unused [236]
>> time.h(70): warning: static function bintime_add unused [236]
>> time.h(82): warning: static function bintime_sub unused [236]
>> time.h(108): warning: static function bintime2timespec unused [236]
>> time.h(116): warning: static function timespec2bintime unused [236]
>> time.h(125): warning: static function bintime2timeval unused [236]
>> time.h(133): warning: static function timeval2bintime unused [236]
>> time.h(152): warning: struct sigevent never defined [233]
>> lint: cannot find llib-lc.ln
>
> These warnings are due to different sets of bugs in lint:
> - lint still doesn't understand the C90 or earlier feature of incomplete
>  struct types.  Incomplete struct types are useful and are common in
>  headers, so lint shouldn't warn about them.
> - lint still doesn't understand the gnu89/C99 feature of "static inline".
>  This feature is even more useful though less common in headers than
>  incomplete struct types.
> - lint libraries seem to have never been configured quite right, so
>  llib-lc.ln doesn't exist.  FreeBSD only has llib-stdc.ln and
>  llib-posix.ln, and these seem to be far too small to be complete.
>
> Bruce
>

Actually I was using x86:

[gcooper@optimus /devel/ncvs/ports]$ uname -mr
8.0-CURRENT i386

(I'm not 100% sure) but a lot of things could have changed since 5.x,
most notably the defining types of some of the structures. They seem
to now properly align with one another, and if I apply your changes,
gcc (with the default warnings enabled) complains that the shifting
doesn't fit the actual data type size.

-Garrett



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?7d6fde3d0806210854y1c3be9ds1e1836b1c08ee671>