[openib-general] [PATCH 02/17] ehca: module infrastructure

[Heiko J Schick]

Hello Roland,

thanks for your commend. I've changed it in my code.

Roland Dreier wrote:
>  > +EXPORT_SYMBOL(ehca_edeb_mask);
> 
> Why does ehca_edeb_mask need to be exported?  What other module
> accesses it?
> 
>  > +	rblock = kmalloc(PAGE_SIZE, GFP_KERNEL);
>  > +	if (rblock == NULL) {
>  > +		EDEB_ERR(4, "Cannot allocate rblock memory.");
>  > +		ret = -ENOMEM;
>  > +		goto num_ports0;
>  > +	}
>  > +
>  > +	memset(rblock, 0, PAGE_SIZE);
> 
> Use kzalloc instead (this appears a quite a few places).
> 
> +	if ((strcmp(#name, "num_ports") == 0) && (ehca_nr_ports == 1))	   \
> +		len = snprintf(buf, 256, "1");				   \
> +	else								   \
> +		len = snprintf(buf, 256, "%d", rblock->name);		   \
> +									   \
> +	if (len < 0)							   \
> +		return 0;						   \
> +	buf[len] = '\n';						   \
> +	buf[len+1] = 0;							   \
> 
> Why not just do
> 
> +	if ((strcmp(#name, "num_ports") == 0) && (ehca_nr_ports == 1))	   \
> +		return snprintf(buf, 256, "1\n");			   \
> +	else								   \
> +		return snprintf(buf, 256, "%d\n", rblock->name);	   \
> 
> and let snprintf put the newline in for you?  Also, the kernel's
> snprintf() will never return a negative number, so there's no need to
> check that.
> 
>  - R.