On 07/25/2011 12:31 PM, Michal Hocko wrote:
> [Please use reply to all]
>
> On Mon 25-07-11 07:43:07, Nikiforov Alex wrote:
>> On 07/22/2011 06:11 PM, Michal Hocko wrote:
>>> On Fri 22-07-11 08:53:53, Nikiforov Alex wrote:
>>>> From f1390d329b1172bfc38b43bb11a56b8cb6db1af8 Mon Sep 17 00:00:00 2001
>>>> From: Alex Nikiforov<nika@gentoo>
>>>> Date: Thu, 21 Jul 2011 13:20:48 +0400
>>>> Subject: [PATCH 1/2] cgclassify: add a PID check into the client
>>>>
>>>> Add PID check code to the client. We dont need any read()/write() if
>>>> it's not valid.
>>>>
>>>> Signed-off-by: Alex Nikiforov<a.nikifo...@samsung.com>
>>>> ---
>>>> src/tools/cgclassify.c | 9 +++++++--
>>>> 1 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/tools/cgclassify.c b/src/tools/cgclassify.c
>>>> index 397b725..cffc63f 100644
>>>> --- a/src/tools/cgclassify.c
>>>> +++ b/src/tools/cgclassify.c
> [...]
>>>> pid = (uid_t) strtol(argv[i],&endptr, 10);
>>>> - if (endptr[0] != '\0') {
>>>> + snprintf(path, FILENAME_MAX, "/proc/%d", pid);
>>>> +
>>>> + /* check PID */
>>>> + if (endptr[0] != '\0' || stat(path,&buff_stat)) {
>>>> /* the input argument was not a number */
>>>> fprintf(stderr, "Error: %s is not valid pid.\n",
>>>> argv[i]);
>>>
>>> I would prefer it like this:
>>> if (!(*endptr))
>>> goto bad_pid;
>>> snprintf(path, FILENAME_MAX, "/proc/%d", pid);
>>> if (stat(path,&buff_stat)) {
>>> bad_pid:
>>> fprintf(stderr, "Error: %s is not valid pid.\n",
>>> argv[i]);
>>>
>>> exit_code = 2;
>>> continue;
>>> }
>>>
>>> Why should we do snprintf if we know that the given parameter was not a
>>> number or it contains some garbage.
>> I really dont like exception style coding in plain C. It's hard to read
>> as well as hard to debug when you add some traces (this is IMHO too). We
>> can add goto, but it's really ugly style here not so much work happen to
>> add goto, snprint() is reenterant in this case it's fast. Dont see any
>> point to add goto here.
>
> You can also create a monster error condition which would be even
> more subtly. The point is, why do you want to burn CPU cycles if you
> know that what you have in pid is not what you want to turn into a
> number. But hey, I am not a maintainer or anybody who can tell that this
> is unacceptable.
I agree with you in the point that when we have huge amount of the
checking we should (or may be must) do it in the exception way. But in
this particular case, sorry Michal but I dont agree with you. We have
pretty simple operation and it's not the price for exception with goto
here, sorry but it's my point. I agree with your point about checking
also on server side it was reasonable, but this dont take any speedup,
just obfuscate the code with goto[some amount with code]label:.
------------------------------------------------------------------------------
Storage Efficiency Calculator
This modeling tool is based on patent-pending intellectual property that
has been used successfully in hundreds of IBM storage optimization engage-
ments, worldwide. Store less, Store more with what you own, Move data to
the right place. Try It Now! http://www.accelacomm.com/jaw/sfnl/114/51427378/
_______________________________________________
Libcg-devel mailing list
Libcg-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libcg-devel
