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
>> @@ -121,7 +121,8 @@ int main(int argc, char *argv[])
>> struct cgroup_group_spec *cgroup_list[CG_HIER_MAX];
>> int c;
>> char *endptr;
>> -
>> + char path[FILENAME_MAX];
>> + struct stat buff_stat;
>>
>> if (argc< 2) {
>> usage(1, argv[0]);
>> @@ -167,8 +168,12 @@ int main(int argc, char *argv[])
>> }
>>
>> for (i = optind; i< argc; i++) {
>> +
>
> Why the empty line?
just dont like tons of code, I think it's very user full to separate
cycle head and the first operator. For me this style is more convenient
when I read a code (this is just IMHO)
>
>> 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.
------------------------------------------------------------------------------
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
