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

[Igor M Podlesny]

On 7 May 2013 20:00, Kir Kolyshkin <kir at openvz.org> wrote:
> 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:
[...]
>>     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.

   <I_know_it_wont_change_your_mind_but_...>
   Kir, it's obvious you should never blindly add code, shouldn't ya?
:) And yep, sometimes it would require you
to change existing code just to conform to new one. It's life in
general, and it's programming in particular. It doesn't mean
you should always restrict yourself to the lowest possible level of
comfort, though.

   What I would agree on is that C++ is a way more convenient in
question, meanwhile C isn't, but alas, it's in C.

   But it doesn't mean we don't have to use convenient things where
they're possible even though we use C.
   </I_know_it_wont_change_your_mind_but_...>
>
>
>>>> -     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.

   Yep, my hope was it was just funny and not offensive in a way
(although there's a thought alike
"I like offending people, because I think people who get offended
should be offended." (c)) ;)
>
> We don't use "yoda conditions" in vzctl code.

   It's a pity...
>
>
>>>>                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
>
>