On Mon, Apr 22, 2013 at 12:23:20PM -0400, Dmitri Pal wrote:
> On 04/22/2013 10:09 AM, Ondrej Kos wrote:
> >>
> >>>       if (ret <= 0 || ret >= 21) {
> >>
> >> you can use sizeof(timestr) here in the odd case timestr actually
> >> changed.
> >
> > quoting the reference:
> > Return Value
> > The number of characters that would have been written if n had been
> > sufficiently large, not counting the terminating null character.
> > If an encoding error occurs, a negative number is returned.
> > Notice that only when this returned value is non-negative and less
> > than n, the string has been completely written.
> > (where N is the sizeof parameter)
> >
> > so I'd rather leave it this way, seems more defensive 
> 
> I am not sure it is more defensive.
> 
> If you do not want to to use sizeof I am fine but I would at least use:
> 
> 
> #define TIME_STRING_SIZE    21
> 
> ...
> 
> char timebuf[TIME_STRING_SIZE];
> 
> ...
> 
> if (ret <= 0 || ret >= TIME_STRING_SIZE) {
> 
> ...
> 
> Seems more defensive to me that literally "21".

Yes, that's pretty much what I meant. sizeof would be OK too, since the
array is in the same scope:

if (ret <= 0 || ret >= sizeof(timebuf)) {
 /* Error */
}

But as Dmitri said, using magic constans is calling for trouble. Maybe
not in this code, because the size is unlikely to change, but let's
follow best practices everywhere :-)
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to