[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: memstat-0.9 hurd patch (PATH_MAX)



2012/1/23 Tanguy LE CARROUR <tanguy.lecarrour@gmail.com>:
> 2012/1/22 Jérémie Koenig <jk@jk.fr.eu.org>
>> The usual trick for "%d" is to use the constant "sizeof (int) * 3 +
>> 1". I included + 1 for the sign, but it's not really necessary if we
>> exepect sizeof(int) >= 2, which we probably should.
>
> Why '*3'? Do you expect PID to be in a certain range? Or is it C-voodoo?!

log(MAX_INT)
  = log(2 ^ 32)
  = 32 * log(2)
  = 4 * 8 * log(2)
  = sizeof(int) * 2.40823997
  < sizeof(int) * 3

>> Also, I think there are some problems with your pointer arithmetic.
>> For instance,
>>      if (fgets(buf+last,size,f) == NULL)
>> will overwrite the last character read from the previous iteration (if
>> there was one).
>
> According to the 'gets' page, each "read" returns a '\0' terminated string.
> Each time I get rid of this '\0' when concatenating.

Terminating strings with '\0' is the standard practice in C; the
string length returned by strlen() does not include the terminating
nul.

>> Last, but not least, you should be careful with whitespace. Your patch
>> mixes tabs and spaces for indentation,
>
> ... which is perfectly consistent with the original code! ;-)
>  Is a mix (maybe with a logic... I haven't figured it out yet) of spaces and
> tabs. I can try to reproduce the same "pattern" but not sure what the rule
> is!

One common mixed approach is to use tabs for the "initial indent" and
use spaces for "supplementary indent" when a line is broken because of
a long statement. Also, I think your editor displays tabs as 4 spaces,
whereas 8 spaces is a more common setting for C code. (but the above
convention should ensure the code looks good regardless).

I don't know what memstat uses. The code might well be inconsistent in
which case you should evaluate which rule is used more often and stick
to that.

Bye,
-- 
Jérémie Koenig <jk@jk.fr.eu.org>
http://jk.fr.eu.org/


Reply to: