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

[Kinkie]

On Fri, Jul 10, 2015 at 2:39 PM, Amos Jeffries <squid3 at treenet.co.nz> wrote:

> 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.
>

Gah, right. #0-ed the whole function body


> 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
>

I've used a variable for readability; I can refactor no problem. As the
variable is const, I expect it to be optimized away anyway. Do you wish me
to?


> in src/debug.cc (or anywhere else) please dont use xstrerror() anymore.
>  - use xstrerr(errno) instead. since the debugs() initilization
> overwrites errno value.
>

Done


>  - 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).
>

via std::cerr or some other mechanism?


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

Done.


> 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.
>

std::endl flushes the output buffer (from
http://en.cppreference.com/w/cpp/io/manip/endl :
"Inserts a newline character into the output sequence os and flushes it as
if by calling os.put(os.widen('\n')) followed by os.flush()."


> thats all I think. thank you.
>



-- 
    Francesco
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squid-cache.org/pipermail/squid-dev/attachments/20150710/6ff6c284/attachment.html>