<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jul 10, 2015 at 3:40 PM, Amos Jeffries <span dir="ltr"><<a href="mailto:squid3@treenet.co.nz" target="_blank">squid3@treenet.co.nz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 11/07/2015 1:21 a.m., Kinkie wrote:<br>
> On Fri, Jul 10, 2015 at 2:39 PM, Amos Jeffries <<a href="mailto:squid3@treenet.co.nz">squid3@treenet.co.nz</a>> wrote:<br>
><br>
>> On 10/07/2015 3:18 a.m., Kinkie wrote:<br>
>>> Hi,<br>
>>>   I'm going through the issues identified by Coverity Scan, in<br>
>>> chronological order.<br>
>>> This patch covers 11 defects, mostly unchecked return values. It also<br>
>>> converts unlinkd to c++ (original defect: using tainted strings). They<br>
>> have<br>
>>> all build-tested; unlinkd has been (hand)-run tested.<br>
>>><br>
>><br>
>> Audited on IRC, summary below:<br>
>><br>
>> in src/DiskIO/AIO/AIODiskIOStrategy.cc does the change actually do<br>
>> anything useful?<br>
>>  - the entire function is dead code due to the return-0 that starts it.<br>
>>  - thats why I marked that issue as intentional to be ignored.<br>
>>  - Ive been contemplating making it #if 0 wrapped for coveritys pleasure.<br>
>><br>
><br>
> Gah, right. #0-ed the whole function body<br>
><br>
><br>
>> in src/DiskIO/DiskDaemon/DiskdFile.cc I thought you were going to use<br>
>> the unlock() result inline for the if-statement, the variable seems<br>
>> unnecessary since its not even used in the debug statement<br>
>><br>
><br>
> I've used a variable for readability; I can refactor no problem. As the<br>
> variable is const, I expect it to be optimized away anyway. Do you wish me<br>
> to?<br>
><br>
><br>
>> in src/debug.cc (or anywhere else) please dont use xstrerror() anymore.<br>
>>  - use xstrerr(errno) instead. since the debugs() initilization<br>
>> overwrites errno value.<br>
>><br>
><br>
> Done<br>
><br>
<br>
</div></div>Meh I mucked up there. THat wont work due to same reason for not using<br>
xstrerror().<br>
<br>
Use a const int xerrno = errno; first, then inside the debugs( ...<br>
xstrerr(xerrno) ...<br></blockquote><div><br></div><div>Doing that in debug.cc; about the others I'll wait for the discussion to reach a conclusion.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
>>  - also at that point in src/debug.cc the debugs() log file is closed.<br>
>>   = at least on Windows builds. have to report errors to stderr instead<br>
>> until we fix debugs() to store in a temporary buffer when waiting on<br>
>> cache.log existence (that could be an XXX comment).<br>
>><br>
><br>
> via std::cerr or some other mechanism?<br>
><br>
><br>
<br>
</span>Good question.<br>
<br>
Looking at it closer I see you were only altering inside the for-loop.<br>
Both the loop and the if-statement need the same checks and output on<br>
failure.<br>
It is probably best to add a helper static function that calls<br>
_db_print_stderr(), _db_print_file(), then _db_print_syslog() in that<br>
specific order with the same arguments to each.<br>
Just be careful not to pass va_args around as they are one-use objects.<br></blockquote><div><br></div><div>I'm not sure it makes sense: in main, if Debug::log_stderr < 0, stderr gets redirected to /dev/null. Andif it isn't, debugs() already prints to stderr via _db_print_stderr; I'm leaving it as-is for now, <br><br></div><div>[...]<br></div><div>  <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+1 with the above extra changes.<br></blockquote><div><br></div><div>Thanks <br></div></div><br></div><br><div class="gmail_extra">-- <br><div class="gmail_signature">    Francesco</div>
</div></div>