[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. -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ------------------------------------------------------------------------------ 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