You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe Jr." <wr...@rowe-clan.net> on 2010/10/11 23:52:41 UTC
logging patch bug
Why were ap_errorlog_format_item->min_loglevel and ap_errorlog_info->level declared
with two different types?
Re: logging patch bug
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 10/13/2010 4:46 PM, Philip Prindeville wrote:
> On 10/12/10 12:57 PM, Stefan Fritsch wrote:
>> On Monday 11 October 2010, William A. Rowe Jr. wrote:
>>> Why were ap_errorlog_format_item->min_loglevel and
>>> ap_errorlog_info->level declared with two different types?
>> ap_errorlog_info->level is -1 if no log level is available, i.e. when
>> logging per-request/per-conn info. However, ap_errorlog_format_item-
>>> min_loglevel always contains a genuine loglevel (which is>=0).
>> Therefore I have declared the former as signed int and the latter as
>> unsigned int.
>>
>> But the comparison (item->min_loglevel> info->level) in log.c does
>> cause a warning with -Wextra, even if it is correct. What do you
>> suggest? Adding a cast or changing the declaration of min_loglevel to
>> signed int?
>
> If they are only compared in one place, I'd use a cast. Least churn, and it means that
> static analysis could still be used elsewhere to easily detect when out-of-range
> assignments are made to the latter.
In any case, this should be fixed before the tag, since it's been almost 2 mos here...
.\server\log.c(1020) : warning C4018: '>' : signed/unsigned mismatch
Re: logging patch bug
Posted by Philip Prindeville <ph...@redfish-solutions.com>.
On 10/12/10 12:57 PM, Stefan Fritsch wrote:
> On Monday 11 October 2010, William A. Rowe Jr. wrote:
>> Why were ap_errorlog_format_item->min_loglevel and
>> ap_errorlog_info->level declared with two different types?
> ap_errorlog_info->level is -1 if no log level is available, i.e. when
> logging per-request/per-conn info. However, ap_errorlog_format_item-
>> min_loglevel always contains a genuine loglevel (which is>=0).
> Therefore I have declared the former as signed int and the latter as
> unsigned int.
>
> But the comparison (item->min_loglevel> info->level) in log.c does
> cause a warning with -Wextra, even if it is correct. What do you
> suggest? Adding a cast or changing the declaration of min_loglevel to
> signed int?
If they are only compared in one place, I'd use a cast. Least churn, and it means that static analysis could still be used elsewhere to easily detect when out-of-range assignments are made to the latter.
Re: logging patch bug
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Monday 11 October 2010, William A. Rowe Jr. wrote:
> Why were ap_errorlog_format_item->min_loglevel and
> ap_errorlog_info->level declared with two different types?
ap_errorlog_info->level is -1 if no log level is available, i.e. when
logging per-request/per-conn info. However, ap_errorlog_format_item-
>min_loglevel always contains a genuine loglevel (which is >=0).
Therefore I have declared the former as signed int and the latter as
unsigned int.
But the comparison (item->min_loglevel > info->level) in log.c does
cause a warning with -Wextra, even if it is correct. What do you
suggest? Adding a cast or changing the declaration of min_loglevel to
signed int?