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

[Kir Kolyshkin]

On 05/07/2013 03:17 AM, Igor M Podlesny wrote:
> 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?

Consider the following scenario

1 We have code like

FILE *fd = fopen(PROCPU, "r");

2 We are adding parsing of another file

FILE *fd2 = fopen(PROCCPU2, "r");

3 Now we are in trouble -- each fopen() can set errno, and we don't have 
a way to analyse it.

>>> -     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? )

Very funny indeed, taking into account that Andrey is a kernel developer.

Igor, please don't get personal on a public mailing list. Attack the 
problem, not the person. Andrey is trying to help you.

We don't use "yoda conditions" in vzctl code.

>>>                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
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel