[Devel] [PATCH 10/10] get_num_cpu(): refactored

Igor M Podlesny openvz at poige.ru
Tue May 7 03:17:17 PDT 2013


On 7 May 2013 16:36, Andrew Vagin <avagin at parallels.com> wrote:
> On Tue, May 07, 2013 at 02:42:12PM +0800, Igor Podlesny wrote:
[…]
>> +     FILE *fd = fopen(PROCCPU, "r");
>
> Usually if a function can return an error, it does not used in
> declarations.

   It's declaration And initialization, BTW. I never saw such a
prejudice regarding dec. & initialization before. Have you some
references?
>
>>
>> -     if ((fd = fopen(PROCCPU, "r")) == NULL) {
>> +     if (NULL == fd) {
>
> Pls, follow the current coding style.
>         if (fd === NULL)

   Somebody suffers from too much PHP, too much, doesn't he? )
>
>>               logger(-1, errno, "Cannot open " PROCCPU);
>> -             return 1;
>> -     }
>> -     while (fgets(str, sizeof(str), fd)) {
>> -             char const proc_ptrn[] = "processor";
>> -             if (!strncmp(str, proc_ptrn, sizeof(proc_ptrn) - 1))
>> -                     ncpu++;
>> +     } else {
>
> Why do we need one more indent here instead of returning 1 in the
> previous block? I am not sure that this version is more readable...

   You might be unsure because patch is difficult to compare,
side-by-side comparison is more convenient or what is even more easier
to see is complete version:

      https://bitbucket.org/poige/vzctl/src/17401655f7993821b4244b7d3291743f6dd3a75f/src/lib/util.c#cl-539

   — please, take a look.

>> +             char str[128];
>> +             while (fgets(str, sizeof(str), fd)) {
>> +                     char const proc_ptrn[] = "processor";
>> +                     if (!strncmp(str, proc_ptrn, sizeof(proc_ptrn) - 1))
>> +                             ncpu++;
>> +             }
>> +             fclose(fd);
>>       }
>> -     fclose(fd);
>> -     return !ncpu ? 1 : ncpu;
>> +     return MAX_OF(1, ncpu);
>>  }
>>
>>  int get_lowmem(unsigned long long *mem)
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Devel mailing list
>> Devel at openvz.org
>> https://lists.openvz.org/mailman/listinfo/devel




More information about the Devel mailing list