Patrice Dumas wrote:
>>> filesys.c: In function 'kdbSetKey_filesys':
>>> filesys.c:286: warning: ignoring return value of 'fchown', declared with
>>> attribute warn_unused_result
>> Probably easily fixed with
>>
>> (void) fchown(blah);
>
> It doesn't... I am not that much worried by those warnings... as long as
> the code maintainers take them into account.
>
>>> kdb.c: In function 'kdbOpenBackend':
>>> kdb.c:321: warning: ISO C forbids conversion of object pointer to function
>>> pointer type
>> This is because the cast is invalid. The kdbLibSym() function returns a
>> (void *), but function pointers may technically be incompatible with
>> that. One way to fix it might be to wrap the function pointer in a
>> "struct Something { ... } x" in the .so. (But I'm sure there are better
>> ways of loading function pointers from a .so. I just don't care enough
>> to look them up.)
>
> I have modified the code (looking at man dlopen), now it looks like
> *(void **)(&kdbBackendFactory)=kdbLibSym(dlhandle,
> "kdbBackendFactory");
> However now there is a new warning:
> kdb.c:321: warning: dereferencing type-punned pointer will break
> strict-aliasing rules
>
Yup, it's basically the same problem in another form. Now gcc complains
that you're creating two pointers of different types pointing to the
same memory location, thus breaking the strict aliasing rules (see
below). The pointer types _might_ be incompatible. IIRC, the C standard
permits sizeof(func *) != sizeof(void *).
>>> kdb.c: In function 'kdbMonitorKeys_default':
>>> kdb.c:860: warning: implicit declaration of function 'usleep'
>>> kdb.c: In function 'kdbInfoToString':
>>> kdb.c:1270: warning: implicit declaration of function 'snprintf'
>> This _should_ be declared by #include <stdio.h>, but it may require
>> some #define _BSD_SOURCE or _GNU_SOURCE or some such.
>
> In fact the -ansi option of gcc that triggers those warnings.
Yup, that sounds about right now that I think about it.
> They might be there because usleep and snprintf aren't in
> all the standard libraries. It may be fixed by defines,
> but removing -ansi seems simpler to me... However on systems
> without those functions there will be issues.
The snprintf man page seems to indicate that snprintf was added in
ISO/C99, so adding --std=c99 may help. However, that again leads to
potential problems because gcc doesn't fully support ISO/C99.
[--snip--]
>
>>> berkeleydb.c: In function 'keyFromBDB':
>>> berkeleydb.c:292: warning: dereferencing type-punned pointer will break
>>> strict-aliasing rules
>> The (char **) cast may simply not be used here. The call needs to use a
>> temporary,
>
> This will also lead to an invalid cast?
Well, all the C standard (can't remember which one ;)) says is that with
strict aliasing, the compiler may assume two pointers of different
types, say
struct S *s;
struct T *t;
always point to non-overlapping locations in memory. I think the (char
**) cast violates that with respect to the formal parameter of UTF8Engine.
I may have a more thorough look at the code tomorrow to see what can be
done.
> What should the code look like?
>
>> or the UTF8Engine declaration needs to be changed. Btw, why
>> isn't the backend using the actual API functions for the Key struct here
>> instead of poking around directly in the Key struct?
>
> Avi or Yannick?
>
[--snip--]
>>> sig.c: In function 'sig_block':
>>> sig.c:44: warning: 'sigblock' is deprecated (declared at
>>> /usr/include/signal.h:181)
>> This code doesn't actually seem to be used anywhere... and probably
>> shouldn't except to turn misc. signals off. Signals are inherently very
>> unportable -- they have very subtly different behavior on different systems.
>
> This has been 'fixed' by detecting if sigprocmask is present, and if so,
> use it. Does the portability issues you are referring to happen with
> sigprocmask and friends or only to the alternative with signal()/sigsetmask?
I should have qualified my statement. What I'm referring to is more of a
semantics thing. Signals behave -- or at least used to, it's been a
while -- differently on BSD, Linux and SunOS in various incompatible
ways. I've long since forgotten the details. Things *may* have improved,
though I wouldn't count on it.
>
>> [--snip--]>
>>> kdbd.c:129: warning: format '%d' expects type 'int', but argument 4 has
>>> type 'pthread_t'
>
> This was fixed by replacing %d by %ld. I don't know if it is the right way...
As long as phread_t == long int, I guess that'll work. I'm not sure if
it'll work on x86_64, though. I'll see if I get time to check out a
fresh copy tomorrow to test.
>
>> On my gcc 4.1.1 on my AMD64 there are also some warnings with
>>
>> format '%d' expects type 'int', but argument bla has type 'size_t'
>
> Where are those warnings located in the code?
I'll see if I can pinpoint them for you tomorrow.
>
>> which, according to the snprintf man page can be fixed by using "%z" in
>> the format string. This is unportable, though, and the parameters should
>> probably just be cast to (int) as it's unlikely to cause any real problems.
>
> Isn't it possible to have size_t wider than int (I am ignorant on on that
> subject...)?
I belive so, but I don't think it is likely to cause problems in these
particular cases since all the values being printed (such as key sizes,
etc.) are probably well within the value of an int.
>
>> The macro KEY_METAINFO_SIZE also has issues on my AMD64:
>>
>> #define KEY_METAINFO_SIZE(k) ((unsigned int)&(k->recordSize) -
>> unsigned int)k)
>>
>> is incorrect when the result is used as a "size_t" in various locations.
>> The correct definition is
>>
>> #define KEY_METAINFO_SIZE(k) ((size_t)&(k->recordSize) - (size_t)k)
>
> Since recordSize is size_t in _Key, wouldn't it be even better to
> use
> #define KEY_METAINFO_SIZE(k) ((k->recordSize) - (size_t)k)
>
Yup, I hadn't checked the definition of recordSize.
Cheers,
--
Bardur Arantsson
<[EMAIL PROTECTED]>
- Facts are useless. You can use facts to prove anything that's
even remotely true.
Homer Simpson, 'The Simpsons'
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Registry-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/registry-list
