Re: (int)sizeof in smtpd

2014-05-08 Thread Ted Unangst
On Thu, May 08, 2014 at 18:43, Alexandre Ratchov wrote:
> On Thu, May 08, 2014 at 12:35:56PM -0400, Ted Unangst wrote:
>> This is wrong in several ways.
>> 
>> Never cast sizeof down, always cast the comparison variable up.
>> 
>> I'll specifically call out this change:
>> 
>> -if (snprintf(buf, sizeof(buf)) >= (int)sizeof(buf))
>> +if ((size_t)snprintf(buf, sizeof(buf)) >= sizeof(buf))
>> 
> 
> note that the >= operator promotes int to size_t, which makes the
> cast useless and could permit lines to be shortened.

Yes, I did that first, but somebody added -Wsign-compare to the
Makefile. Silencing that warning is the root of a lot of evil. The
cure is usually worse than the disease.



Re: (int)sizeof in smtpd

2014-05-08 Thread Franco Fichtner
On 08 May 2014, at 18:43, Alexandre Ratchov  wrote:

> On Thu, May 08, 2014 at 12:35:56PM -0400, Ted Unangst wrote:
>> This is wrong in several ways.
>> 
>> Never cast sizeof down, always cast the comparison variable up.
>> 
>> I'll specifically call out this change:
>> 
>> -if (snprintf(buf, sizeof(buf)) >= (int)sizeof(buf))
>> +if ((size_t)snprintf(buf, sizeof(buf)) >= sizeof(buf))
>> 
> 
> note that the >= operator promotes int to size_t, which makes the
> cast useless and could permit lines to be shortened.

Wouldn't that produce a signed vs. unsigned comparison?



Re: (int)sizeof in smtpd

2014-05-08 Thread Alexandre Ratchov
On Thu, May 08, 2014 at 12:35:56PM -0400, Ted Unangst wrote:
> This is wrong in several ways.
> 
> Never cast sizeof down, always cast the comparison variable up.
> 
> I'll specifically call out this change:
> 
> - if (snprintf(buf, sizeof(buf)) >= (int)sizeof(buf))
> + if ((size_t)snprintf(buf, sizeof(buf)) >= sizeof(buf))
> 

note that the >= operator promotes int to size_t, which makes the
cast useless and could permit lines to be shortened.