[PATCH 2/3] Config file fixes.

Richard Kettlewell rjk at terraraq.org.uk
Fri Aug 5 22:17:07 BST 2011


On 04/08/2011 15:16, Ian Jackson wrote:
> Richard Kettlewell writes ("[PATCH 2/3] Config file fixes."):
>>   /* Convert a node type to a string, for parse tree dump */
>> -static string_t ntype(uint32_t type)
>> +static const char *ntype(uint32_t type)
>
> We seem to be gradually abolishing [c]string_t.  Perhaps we should do
> so globally ?  (Says the guy with a huge outstanding patch serieses...)

I'll add it to my list. l-)

>> @@ -197,7 +197,7 @@ static void ptree_dump(struct p_node *n, uint32_t d)
>>   	default:       printf("**unknown primitive type**\n"); break;
>>   	}
>>       } else {
>> -	assert(d<INT_MAX);
>> +	assert(d<10000);
>
> This is an excessive recursion catch, isn't it ?  Why is it a uint32_t
> even ?  I think my uint32_t =>  int changes should probably have
> touched this too.

Oh, yes, int would be much better.

>> diff --git a/conffile.fl b/conffile.fl
>> index 2cfa21b..7228c9e 100644
>> --- a/conffile.fl
>> +++ b/conffile.fl
> ...
>> -	uint32_t lineno;
>> +	int lineno;
>
> I remember spotting this and not changing it.  I can't remember why.

I've skimmed its uses again and still can't see any reason not to make 
it 'int'.  It's assigned from config_lineno which is an int in current 
HEAD, so it's not like it gives you an extra 2 billion lines of elbow 
room (in case you want to use the config infrastructure to its true 
potential l-)

>> @@ -71,8 +71,25 @@ static struct p_node *stringnode(string_t string)
>>   static struct p_node *numnode(string_t number)
>>   {
>>   	struct p_node *r;
>> +	unsigned long n;
>>   	r=leafnode(T_NUMBER);
>> -	r->data.number=atoi(number);
>> +	errno = 0;
>> +	n = strtoul(number, NULL, 10);
>> +	/* The caller is expected to only give us [0-9]+,
>> +	 * so we skip some of the usual syntax checking. */
>
> By "the caller" you mean flex ?

Yes; the only caller of numnode[1] is in conffile.fl:

[[:digit:]]+		yylval=numnode(yytext); return TOK_NUMBER;

If we ever call it from elsewhere with inadequately constrained input 
then the comment would become false.  I can add the extra checking if 
you think that's worthwhile.

   [1] or 'duodenum', as tbird's spelling checker would prefer.

ttfn/rjk



More information about the sgo-software-discuss mailing list