[PATCH 11/11] staging: dgap: Fix various previously missed checkpatch errors

[Mark Hounschell]

On 02/28/2014 05:36 PM, Dan Carpenter wrote:
> Please redo this one.
>
> On Fri, Feb 28, 2014 at 03:49:09PM -0500, Mark Hounschell wrote:
>> This patch fixes various small checkpatch errors
>> I missed in patches 01-10.
>>
>> Signed-off-by: Mark Hounschell <markh at compro.net>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> ---
>>   drivers/staging/dgap/dgap.c | 16 ++++++----------
>>   1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
>> index 533a9e4..e1dc894 100644
>> --- a/drivers/staging/dgap/dgap.c
>> +++ b/drivers/staging/dgap/dgap.c
>> @@ -260,7 +260,7 @@ static uint dgap_driver_start = FALSE;
>>   static struct class *dgap_class;
>>
>>   static struct board_t *dgap_BoardsByMajor[256];
>> -static uchar *dgap_TmpWriteBuf = NULL;
>> +static uchar *dgap_TmpWriteBuf;
>>   DECLARE_MUTEX(dgap_TmpWriteSem);
>>   static uint dgap_count = 500;
>>
>> @@ -733,12 +733,8 @@ static void dgap_cleanup_board(struct board_t *brd)
>>   	}
>>
>>   	/* Free all allocated channels structs */
>> -	for (i = 0; i < MAXPORTS ; i++) {
>> -		if (brd->channels[i]) {
>> -			kfree(brd->channels[i]);
>> -			brd->channels[i] = NULL;
>> -		}
>> -	}
>> +	for (i = 0; i < MAXPORTS ; i++)
>> +		kfree(brd->channels[i]);
>
> Does checkpatch complain about this, really?  This is a code change and
> needs to be explained in the commit message at least.
>

Yes. It complained:
WARNING: kfree(NULL) is safe this check is probably not required
#745: FILE: drivers/staging/dgap/dgap.c:745:
+               if (brd->channels[i]) {
+                       kfree(brd->channels[i]);

I saw no reason to leave the brd->channels[i] = NULL; there.

>>
>>   	kfree(brd->flipbuf);
>>   	kfree(brd->flipflagbuf);
>> @@ -761,7 +757,7 @@ static int dgap_found_board(struct pci_dev *pdev, int id)
>>
>>   	/* get the board structure and prep it */
>>   	brd = dgap_Board[dgap_NumBoards] =
>> -	(struct board_t *) kzalloc(sizeof(struct board_t), GFP_KERNEL);
>> +		kzalloc(sizeof(struct board_t), GFP_KERNEL);
>>   	if (!brd)
>>   		return -ENOMEM;
>
> This is ugly.  Do:
>
> 	brd = kzalloc(sizeof(struct board_t), GFP_KERNEL);
> 	if (!brd)
> 		return -ENOMEM;
>
> 	dgap_Board[dgap_NumBoards] = brd;
>
> Except really that assignment should happen after the other allocations
> which can fail.  Otherwise we end up with a freed pointer in
> dgap_Board[dgap_NumBoards] on error.
>
>
>>
>> @@ -7415,7 +7411,7 @@ static int	dgap_parsefile(char **in, int Remove)
>>    * the group, and returns that position.  If the first character is a ^, then
>>    * this will match the first occurrence not in that group.
>>    */
>> -static char *dgap_sindex (char *string, char *group)
>> +static char *dgap_sindex(char *string, char *group)
>>   {
>>   	char    *ptr;
>>
>> @@ -7462,7 +7458,7 @@ static int dgap_gettok(char **in, struct cnode *p)
>>   		dgap_err("board !!type not specified");
>>   		return 1;
>>   	} else {
>> -		while ( (w = dgap_getword(in)) != NULL ) {
>> +		while ((w = dgap_getword(in)) != NULL) {
>
> 		while ((w = dgap_getword(in))) {
>
> Double negatives hurt your brain cells for no reason.  (This has been
> proven by science).
>
>>   			snprintf(dgap_cword, MAXCWORD, "%s", w);
>>   			for (t = dgap_tlist; t->token != 0; t++) {
>>   				if (!strcmp(w, t->string))
>

Ok. I'll redo this one next week also.

Thanks
Mark