[squid-dev] [PATCH] Fix various issues detected by Coverity Scan

Amos Jeffries squid3 at treenet.co.nz
Fri Jul 10 12:39:33 UTC 2015


On 10/07/2015 3:18 a.m., Kinkie wrote:
> Hi,
>   I'm going through the issues identified by Coverity Scan, in
> chronological order.
> This patch covers 11 defects, mostly unchecked return values. It also
> converts unlinkd to c++ (original defect: using tainted strings). They have
> all build-tested; unlinkd has been (hand)-run tested.
> 

Audited on IRC, summary below:

in src/DiskIO/AIO/AIODiskIOStrategy.cc does the change actually do
anything useful?
 - the entire function is dead code due to the return-0 that starts it.
 - thats why I marked that issue as intentional to be ignored.
 - Ive been contemplating making it #if 0 wrapped for coveritys pleasure.


in src/DiskIO/DiskDaemon/DiskdFile.cc I thought you were going to use
the unlock() result inline for the if-statement, the variable seems
unnecessary since its not even used in the debug statement


in src/debug.cc (or anywhere else) please dont use xstrerror() anymore.
 - use xstrerr(errno) instead. since the debugs() initilization
overwrites errno value.
 - also at that point in src/debug.cc the debugs() log file is closed.
  = at least on Windows builds. have to report errors to stderr instead
until we fix debugs() to store in a temporary buffer when waiting on
cache.log existence (that could be an XXX comment).


in src/ipc.cc the conventions Ive been using is to name the sockopt
which failed. ie debugs(..."setsockopt(FOO) failed: " << xstrerr(errno))


in src/unlinkd_daemon.cc #include statements aphabetical please. helps
avoid a reformat followup commit.
- can you double-check the removal of setbuf() please? AFAIK the
iostreams still need un-buffering and theres no iostreams way to do it.


thats all I think. thank you.


Amos


More information about the squid-dev mailing list