Factorization of some code.

Theodore Papadopoulo Theodore.Papadopoulo@sophia.inria.fr
Mon Jan 18 14:20:00 GMT 1999


>   > + /* Decode the string p as an integral parameter.
>   > +    If the string is indeed an integer assign its numeric value into
>   > +    the varibale pointed by valuep else issue an Invalid Option error
>   > +    for the option pname. */
>   > +    
>   > + void
>   > + read_integral_parameter (p, valuep, pname)
>   > +      char *p;
>   > +      int  *valuep;
>   > +      char *pname;
> 
> Instead of passing a pointer to a value, I'd have this function just return
> the value.  In the case of an error, it won't matter what value you return
> (use zero).

I'd use -1 instead of 0 as 0 might be a valid parameter (think of -O0 
for example (even if I'm not sure that the function 
read_integral_parameter can fully apply to the -O switches)).

One drawback of having the value returned is that then this value has 
to be checked for each call in order to restore some reasonnable 
default. This leads to the following style:

 	  int value = read_integral_parameter (p + 15, p - 2);
 	  if (value != -1)
 	    max_tinst_depth = value;

So I would propose to do the following instead:

int
read_integral_parameter (p, pname, default)
char *p;
char *pname;
int default;

which will decode the string pointed at by P and return its numeric 
value. If there is a syntax error, then if will emit a message
"Unknown option PNAME" and will return default.
This would lead to the following style which seems better to me.

max_tinst_depth = read_integral_parameter (p + 15, p - 2, max_tinst_depth);

What do you think ???


--------------------------------------------------------------------
Theodore Papadopoulo
Email: Theodore.Papadopoulo@sophia.inria.fr Tel: (33) 04 92 38 76 01
 --------------------------------------------------------------------





More information about the Gcc-patches mailing list