[ccan] Ccanlint Patch

[Rusty Russell]

On Fri, 19 Jun 2009 08:22:53 am Idris S wrote:
> Hello Rusty,
>
> The patch is attached. Thanks

Hi Idris,

   This is better than your previous attempt, but still flawed:

	$ tools/ccanlint/ccanlint -d ccan/alloc
	Segmentation fault
	$

> @@ -104,7 +113,7 @@
>  
>  	if (i->handle)
>  		i->handle(m, result);
> -
> +	
>  	return false;
>  }
>  

Minor point: gratuitous whitespace change.

> @@ -113,14 +122,19 @@
>  	va_list ap;
>  	struct ccanlint *depends; 
>  	struct dependent *dchild;
> +	
> +	if(!test->total_score)
> +		list_add(&compulsory_tests, &test->list);
> +	else
> +		list_add(&normal_tests, &test->list);
>  
> -	list_add(&tests, &test->list);
> +	list_add(&tests, &test->list); //general list

You can't add to two lists!  We don't need the general list at all any more.

> +/**
> + * get_next_test - retrieves the next test to be processed, also
> + * 		    checks whether or not test has a dependency which is zero
> + *  		with ttype = 0 (compulsory test) else it is a normal test  
> + **/
> +static inline struct ccanlint *get_next_test(int ttype)
> +{ 

This is much neater if you simply hand the list_head into the function.

> +	const struct ccanlint  *i;
> +	const struct dependent *d;
> +	const struct list_head *tt;
> +	int start = 0;
> +
> +	tt = (ttype) ? &normal_tests : &compulsory_tests; 	
> +	list_for_each(tt, i, list){
> +	    start = i->num_depends;
> +	    list_for_each(tt, d, node){
> +		if(d->dependent->num_depends != 0)
> +		    start--;
> +		else 
> +		    return d->dependent; //this is the iteration    
> +	    }
> +	    if(start == 0){ //cycle occured 
> +		if(verbose) 
> +		printf("%s could not find test with num_depends=0\n"
> +		       "Error was in test %s\n",__func__, i->name);
> +		exit(1);
> +	    }
> +	}
> +	return NULL;

This is more complicated than it needs to be.  How about:

	if (list_empty(tt))
		return NULL;

	list_for_each(tt, i, list) {
		if (i->num_depends == 0)
			return i;
	}
	errx(1, "Can't make process; test dependency cycle");

Hope that clarifies?
Rusty.