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 <ma...@compro.net>
Cc: Greg Kroah-Hartman <gre...@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

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to