On Sun, May 18, 2014 at 07:24:00PM +0200, Patrik Lundin wrote: > > First up is this one: > === > pin.c: In function 'hsm_shm_open': > pin.c:209: warning: comparison between signed and unsigned > === > > Next we have this one: > === > hsmspeed.c:38:1: warning: "PTHREAD_THREADS_MAX" redefined > In file included from hsmspeed.c:33: > /usr/include/pthread.h:55:1: warning: this is the location of the > previous definition > === > > Third and final (for now): > === > hsmspeed.c: In function 'sign': > hsmspeed.c:120: warning: control reaches end of non-void function > === >
Jerry Lundström from the OpenDNSSEC-team has been kind enough to create a branch where he fixed the stuff I posted above, and made it into a pull request: https://github.com/opendnssec/opendnssec/pull/83 I have imported the changes as patches and those warnings are now gone. Since they have been sorted out I'll now go through the the rest of the warnings. Since upstream is watching I will be as verbose as possible. First of all there are a bunch of libraries spitting out warnings on several locations, I consider these outside the scope: === .libs/libxml2.so.15.1: warning: strcpy() is almost always misused, please use strlcpy() .libs/libxml2.so.15.1: warning: rand_r() isn't random; consider using arc4random() .libs/libldns.so.6.1: warning: random() isn't random; consider using arc4random() .libs/libxml2.so.15.1: warning: strcat() is almost always misused, please use strlcat() === Now for the first warning in the code at hand: ../ksm/libksm.a(datetime.o)(.text+0x562): In function `DtSecondsInterval': : warning: strcpy() is almost always misused, please use strlcpy() The code looks like this: enforcer/ksm/datetime.c: char buffer[64]; [...] else { strcpy(buffer, "0s"); } I am not sure trying to turn this into strlcpy is worth it since the string is static and the buffer obviously large enough. (Though I welcome cluebats from anyone more well versed in C here). Next up some random warnings: === ../ksm/libksm.a(ksm_policy.o)(.text+0xd31): In function `KsmPolicyUpdateSalt': : warning: rand() isn't random; consider using arc4random() ../ksm/libksm.a(ksm_policy.o)(.text+0xd13): In function `KsmPolicyUpdateSalt': : warning: srand() seed choices are invariably poor === Now these warnings were interesting, looking into the code it looks like this: enforcer/ksm/ksm_policy.c: === #ifdef HAVE_ARC4RANDOM for (i = 0; i < 2*(policy->denial->saltlength); i++) { salt[i] = hex_chars[arc4random()%strlen(hex_chars)]; } #else srand( time(NULL) ); for (i = 0; i < 2*(policy->denial->saltlength); i++) { salt[i] = hex_chars[rand()%strlen(hex_chars)]; } #endif === It is obviously prepared to use arc4random() if it is available, yet it does not for some reason. I found HAVE_ARC4RANDOM is indeed set: common/config.h: === /* Define to 1 if you have the `arc4random' function. */ #define HAVE_ARC4RANDOM 1 /* Define to 1 if you have the `arc4random_uniform' function. */ #define HAVE_ARC4RANDOM_UNIFORM 1 === And even -DHAVE_CONFIG_H is supplied during the build: === cc -DHAVE_CONFIG_H -I. -I../../common -I../../common -I../../common -I./include -I./include -I/usr/local/include -I/usr/local/include/libxml2 -I/usr/l ocal/include -O2 -pipe -Wall -Wextra -pthread -MT ksm_policy.o -MD -MP -MF .deps/ksm_policy.Tpo -c -o ksm_policy.o ksm_policy.c === However, I finally noticed config.h is never included in ksm_policy.c. I tried doing so before the current #include lines as is done in other files, and it made the warning disappear. There was one more random-related warning: === notify.o(.text+0x233): In function `notify_setup':^M : warning: random() isn't random; consider using arc4random()^M === The call in signer/src/wire/notify.c: random()%(extra-base); I guess #ifdef'ing this with HAVE_ARC4RANDOM like is done in ksm_policy.c would be nice. Next up some strcat stuff: === ../ksm/libksm.a(datetime.o)(.text+0x961): In function `DtAppendTime': : warning: strcat() is almost always misused, please use strlcat() === The DtAppendTime() function in enforcer/ksm/datetime.c contains six calls to strcat, I dont feel comfortable converting these to strlcat, in case i introduce bugs myself. Input is welcome though. And another instance: === kc_helper.o(.text+0x677): In function `StrAppend': : warning: strcat() is almost always misused, please use strlcat() === ... the call in enforcer/utils/kc_helper.c: strcat(*str1, str2); Same thing applies here. Input is welcome. Finally some unused variables, might as well be complete while I am at it: === daemon/cmdhandler.c:398: warning: unused variable 'task' shared/duration.c:442: warning: 'is_leap_year' defined but not used shared/duration.c:449: warning: 'leap_days' defined but not used shared/hsm.c:224: warning: unused variable 'retries' shared/hsm.c:220: warning: unused variable 'status' === Regards, Patrik Lundin
