Skip to content

staging: gdm72xx: Code cleanup

Michalis Pappas lkml 4 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v4 Latest0 of 3 patches 0 repliesNew
  2. v32 of 3 patches 5 repliesSuperseded
  3. v23 patches 2 repliesSuperseded
Earlier versions · 1
VersionPostedPatchesRepliesStatus
v13 patches1 replySuperseded

Patches in v4 No applies recorded yet

None of the 3 patches the cover letter announces are in this archive.

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.

Read the v4 thread

Cover letter [PATCH v4 0/3] staging: gdm72xx: Code cleanup

Ok, I generated a new patchset based on running checkpatch.pl with the --strict option, as well as any recommendations made earlier on this thread.

Quoted 1 line

From the following patches, both 8/10 and 9/10 involve whitespace fixes:

the former fixes issues reported by checkpatch.pl while the latter is mostly indentation stuff found by inspecting the code, stray tabs etc. I am sending them as separate patches in case one wants to keep only one of them.

After applying the patches, the following issues are still reported by checkpatch.pl. My comments follow:

ERROR: Macros with complex values should be enclosed in parenthesis #34: FILE: usb_ids.h:34:
+#define USB_DEVICE_BOOTLOADER(vid, pid) \
+ {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD)}, \
+ {USB_DEVICE((vid), ((pid)&BL_PID_MASK)|B_DOWNLOAD|B_DIFF_DL_DRV)}

We agreed on that one already.

WARNING:LONG_LINE: line over 80 characters

This comes out in cases where I slightly bent the 80 chars rule to improve readability. Should anyone complain that it doesn't fit their terminal I promise I'll revert it back ;)

WARNING: unchecked sscanf return value
#295: FILE: gdm_wimax.c:295:
+ sscanf(e->dev->name, "wm%d", &idx);

Quoted 1 line

From my understanding this should be ok. The value stored in e->dev>name

is generated by __dev_alloc_name which does all the necessary checks for user supplied input etc, so it should be considered as a trusted value.

WARNING:SPACING: Missing a blank line after declarations

This is a false positive, as it refers to pointer-to-function declarations withing structures.

CHECK:BRACES: Blank lines aren't necessary after an open brace '{'

I found it to be more readable the way it was, so I didn't touch it.

CHECK:UNCOMMENTED_DEFINITION: spinlock_t definition without comment

I am not sure what the locks are used for so I am unable to write a comment on these.

Regards,

-Michalis