> -----Original Message-----
> From: Caspar Zhang [mailto:[email protected]]
> Sent: Thursday, February 16, 2012 7:40 AM
> To: Filippo ARCIDIACONO
> Cc: [email protected]
> Subject: Re: [LTP] [PATCH v2] getrusage03: add opportunity to reduce
> memory allocation's size
> 
> Hi,
> 
> On 02/15/2012 09:04 PM, Filippo ARCIDIACONO wrote:
> >  static struct rusage ru;
> >  static long maxrss_init;
> > -static int retval, status;
> > +static int retval, status, opt_factor;
> >  static pid_t pid;
> > +static char *factor_str;
> > +int factor_nr = 10;
> 
> missing `static`?

In my opinion doesn't need to be static.

> 
> >
> > +option_t child_options[] = {
> > +   { "m:", &opt_factor, &factor_str },
> > +   { NULL, NULL,         NULL }
> > +};
> > +
> > +static void usage(void);
> >  static void inherit_fork(void);
> > -static void inherit_fork2(void);
> > +static void inherit_fork2(const int size);
> >  static void fork_malloc(void);
> >  static void grandchild_maxrss(void);
> >  static void zombie(void);
> > @@ -68,23 +76,33 @@ static void cleanup(void);
> >
> >  int main(int argc, char *argv[])
> >  {
> > -   int lc;
> > +   int lc, size;
> >     char *msg;
> >
> > -   msg = parse_opts(argc, argv, NULL, NULL);
> > +   msg = parse_opts(argc, argv, child_options, usage);
> >     if (msg != NULL)
> >             tst_brkm(TBROK, NULL, "OPTION PARSING ERROR - %s", msg);
> >
> >     setup();
> >
> > +   if (opt_factor)
> > +           factor_nr = atoi(factor_str);
> > +
> > +   if (factor_nr == 0)
> > +           tst_brkm(TBROK, cleanup, "Input factor must be != 0");
> 
> Hi, I guess factor_nr should not be < 0 too? If so, you might want to
> use
> 
>     factor_nr = SAFE_STRTOL(NULL, factor_str, 1, LONG_MAX);
> 
> where SAFE_STRTOL is defined in safe_macros.h, of course you should
> make
> factor_nr to be long int.


Factor_nr being a multiply factor, should be a relatively small number,
int type should be enough. For this reason I didn't use the SAFE_STRTOL macro 
you have recently introduced.
It could modify the factor_nr check to "if (factor_nr <= 0)".
Alternatively, as you suggest, it could be use SAFE_STRTOL. In this case I 
suggest something like 
        factor_nr = SAFE_STRTOL(NULL, factor_str, 1, FACTOR_MAX)
where FACTOR_MAX should be locally defined, for example, to 20 (two times of 
actual).

> 
> Rest looks good to me.
> 
> Thanks,
> Caspar

Filippo.


------------------------------------------------------------------------------
Virtualization & Cloud Management Using Capacity Planning
Cloud computing makes use of virtualization - but cloud computing 
also focuses on allowing computing to be delivered as a service.
http://www.accelacomm.com/jaw/sfnl/114/51521223/
_______________________________________________
Ltp-list mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ltp-list

Reply via email to