| Message ID | 1343200277-14385-1-git-send-email-chenbdchenbd@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 2012-07-25 at 15:11 +0800, Baodong Chen wrote: > Fexed coding style issues from scripts/checkpatch.pl in drivers/input Not all checkpatch messages need to be "fexed". :) Please use some taste and judgment before submitting patches based solely on checkpatch output. > diff --git a/drivers/input/apm-power.c b/drivers/input/apm-power.c [] > @@ -33,7 +33,7 @@ static void system_power_event(unsigned int keycode) > } > > static void apmpower_event(struct input_handle *handle, unsigned int type, > - unsigned int code, int value) > + unsigned int code, int value) Perhaps prefer alignment to immediately after open parenthesis. > diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c [] > @@ -547,16 +547,44 @@ static int handle_eviocgbit(struct input_dev *dev, > > switch (type) { > > - case 0: bits = dev->evbit; len = EV_MAX; break; [] > + case 0: > + bits = dev->evbit; > + len = EV_MAX; > + break; not all that better really. > @@ -567,8 +595,8 @@ static int handle_eviocgbit(struct input_dev *dev, > if (type == EV_KEY && size == OLD_KEY_MAX) { > len = OLD_KEY_MAX; > if (printk_timed_ratelimit(&keymax_warn_time, 10 * 1000)) > - pr_warning("(EVIOCGBIT): Suspicious buffer size %u, " > - "limiting output to %zu bytes. See " > + pr_warn("(EVIOCGBIT): Suspicious buffer size %u, " \ > + "limiting output to %zu bytes. See " \ The line continuations are also not necessary and I think are really ugly. Just coalesce the format. > "http://userweb.kernel.org/~dtor/eviocgbit-bug.html\n", dead link I think. > diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c [] > @@ -138,8 +138,8 @@ int input_ff_upload(struct input_dev *dev, struct ff_effect *effect, > > if (effect->id == -1) { > for (id = 0; id < ff->max_effects; id++) > - if (!ff->effect_owners[id]) > - break; > + if (!ff->effect_owners[id]) > + break; Better surrounded by braces too. cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
i want to participate in and to start from fixing simple issues! how could i choose issues form checkpatch's output to find which should be fixed which are not? or checkpatch.pl should be updated?? On Wed, Jul 25, 2012 at 3:30 PM, Joe Perches <joe@perches.com> wrote: > On Wed, 2012-07-25 at 15:11 +0800, Baodong Chen wrote: >> Fexed coding style issues from scripts/checkpatch.pl in drivers/input > > Not all checkpatch messages need to be "fexed". :) > > Please use some taste and judgment before submitting > patches based solely on checkpatch output. > >> diff --git a/drivers/input/apm-power.c b/drivers/input/apm-power.c > [] >> @@ -33,7 +33,7 @@ static void system_power_event(unsigned int keycode) >> } >> >> static void apmpower_event(struct input_handle *handle, unsigned int >> type, >> - unsigned int code, int value) >> + unsigned int code, int value) > > Perhaps prefer alignment to immediately after open parenthesis. > >> diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c > [] >> @@ -547,16 +547,44 @@ static int handle_eviocgbit(struct input_dev *dev, >> >> switch (type) { >> >> - case 0: bits = dev->evbit; len = EV_MAX; break; > [] >> + case 0: >> + bits = dev->evbit; >> + len = EV_MAX; >> + break; > > not all that better really. > >> @@ -567,8 +595,8 @@ static int handle_eviocgbit(struct input_dev *dev, >> if (type == EV_KEY && size == OLD_KEY_MAX) { >> len = OLD_KEY_MAX; >> if (printk_timed_ratelimit(&keymax_warn_time, 10 * 1000)) >> - pr_warning("(EVIOCGBIT): Suspicious buffer size %u, >> " >> - "limiting output to %zu bytes. See " >> + pr_warn("(EVIOCGBIT): Suspicious buffer size %u, " >> \ >> + "limiting output to %zu bytes. See " \ > > The line continuations are also not necessary and I think are > really ugly. Just coalesce the format. > >> >> "http://userweb.kernel.org/~dtor/eviocgbit-bug.html\n", > > dead link I think. > >> diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c > [] >> @@ -138,8 +138,8 @@ int input_ff_upload(struct input_dev *dev, struct >> ff_effect *effect, >> >> if (effect->id == -1) { >> for (id = 0; id < ff->max_effects; id++) >> - if (!ff->effect_owners[id]) >> - break; >> + if (!ff->effect_owners[id]) >> + break; > > Better surrounded by braces too. > > cheers, Joe > > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2012-07-25 at 15:44 +0800, Baodong Chen wrote: > i want to participate in and to start from > fixing simple issues! That's fine Baodong. Welcome. > how could i choose issues form checkpatch's output to find which > should be fixed which are > not? checkpatch output is merely a guide for submission of patches to conform to some generic "kernel style". I think there are times when really ugly code should have a pass to make it more "kernel style" like. Most of those cleanup type changes should be done only to code in the "drivers/staging" directory. I think most of the time, unless you are also fixing some other underlying code defect or shortcoming, checkpatch only cleanups to existing code are best avoided. I suggest for now, until you are really comfortable with the preferred style, you confine yourself to drivers/staging. > or checkpatch.pl should be updated?? You could work on that too. One possibility is to write a utility to take checkpatch output and generate patches automatically, or perhaps extend checkpatch itself to do that. (ie: add a --fix option) cheers, Joe -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 25, 2012 at 4:01 PM, Joe Perches <joe@perches.com> wrote: > On Wed, 2012-07-25 at 15:44 +0800, Baodong Chen wrote: >> i want to participate in and to start from >> fixing simple issues! > > That's fine Baodong. Welcome. > >> how could i choose issues form checkpatch's output to find which >> should be fixed which are >> not? > > checkpatch output is merely a guide for submission of patches > to conform to some generic "kernel style". I think there are > times when really ugly code should have a pass to make it more > "kernel style" like. Most of those cleanup type changes should > be done only to code in the "drivers/staging" directory. > > I think most of the time, unless you are also fixing some other > underlying code defect or shortcoming, checkpatch only cleanups > to existing code are best avoided. > > I suggest for now, until you are really comfortable with the > preferred style, you confine yourself to drivers/staging. > hi,Joe; thank for your explaining! ok, i will start from drivers/staging >> or checkpatch.pl should be updated?? > > You could work on that too. > > One possibility is to write a utility to take checkpatch output > and generate patches automatically, or perhaps extend checkpatch > itself to do that. (ie: add a --fix option) > > cheers, Joe > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Baodong, On Wed, Jul 25, 2012 at 03:11:17PM +0800, Baodong Chen wrote: > Fexed coding style issues from scripts/checkpatch.pl in drivers/input > Thank you for your patch. I picked up the parts that made sense and dropped the rest. Thanks.
diff --git a/drivers/input/apm-power.c b/drivers/input/apm-power.c index e90ee3d..543d85b 100644 --- a/drivers/input/apm-power.c +++ b/drivers/input/apm-power.c @@ -33,7 +33,7 @@ static void system_power_event(unsigned int keycode) } static void apmpower_event(struct input_handle *handle, unsigned int type, - unsigned int code, int value) + unsigned int code, int value) { /* only react on key down events */ if (value != 1) diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c index 6c58bff..a0104bd 100644 --- a/drivers/input/evdev.c +++ b/drivers/input/evdev.c @@ -547,16 +547,44 @@ static int handle_eviocgbit(struct input_dev *dev, switch (type) { - case 0: bits = dev->evbit; len = EV_MAX; break; - case EV_KEY: bits = dev->keybit; len = KEY_MAX; break; - case EV_REL: bits = dev->relbit; len = REL_MAX; break; - case EV_ABS: bits = dev->absbit; len = ABS_MAX; break; - case EV_MSC: bits = dev->mscbit; len = MSC_MAX; break; - case EV_LED: bits = dev->ledbit; len = LED_MAX; break; - case EV_SND: bits = dev->sndbit; len = SND_MAX; break; - case EV_FF: bits = dev->ffbit; len = FF_MAX; break; - case EV_SW: bits = dev->swbit; len = SW_MAX; break; - default: return -EINVAL; + case 0: + bits = dev->evbit; + len = EV_MAX; + break; + case EV_KEY: + bits = dev->keybit; + len = KEY_MAX; + break; + case EV_REL: + bits = dev->relbit; + len = REL_MAX; + break; + case EV_ABS: + bits = dev->absbit; + len = ABS_MAX; + break; + case EV_MSC: + bits = dev->mscbit; + len = MSC_MAX; + break; + case EV_LED: + bits = dev->ledbit; + len = LED_MAX; + break; + case EV_SND: + bits = dev->sndbit; + len = SND_MAX; + break; + case EV_FF: + bits = dev->ffbit; + len = FF_MAX; + break; + case EV_SW: + bits = dev->swbit; + len = SW_MAX; + break; + default: + return -EINVAL; } /* @@ -567,8 +595,8 @@ static int handle_eviocgbit(struct input_dev *dev, if (type == EV_KEY && size == OLD_KEY_MAX) { len = OLD_KEY_MAX; if (printk_timed_ratelimit(&keymax_warn_time, 10 * 1000)) - pr_warning("(EVIOCGBIT): Suspicious buffer size %u, " - "limiting output to %zu bytes. See " + pr_warn("(EVIOCGBIT): Suspicious buffer size %u, " \ + "limiting output to %zu bytes. See " \ "http://userweb.kernel.org/~dtor/eviocgbit-bug.html\n", OLD_KEY_MAX, BITS_TO_LONGS(OLD_KEY_MAX) * sizeof(long)); diff --git a/drivers/input/ff-core.c b/drivers/input/ff-core.c index 480eb9d..f50f6dd 100644 --- a/drivers/input/ff-core.c +++ b/drivers/input/ff-core.c @@ -138,8 +138,8 @@ int input_ff_upload(struct input_dev *dev, struct ff_effect *effect, if (effect->id == -1) { for (id = 0; id < ff->max_effects; id++) - if (!ff->effect_owners[id]) - break; + if (!ff->effect_owners[id]) + break; if (id >= ff->max_effects) { ret = -ENOSPC; diff --git a/drivers/input/ff-memless.c b/drivers/input/ff-memless.c index 5f55885..a237cf2 100644 --- a/drivers/input/ff-memless.c +++ b/drivers/input/ff-memless.c @@ -72,12 +72,12 @@ static const struct ff_envelope *get_envelope(const struct ff_effect *effect) static const struct ff_envelope empty_envelope; switch (effect->type) { - case FF_PERIODIC: - return &effect->u.periodic.envelope; - case FF_CONSTANT: - return &effect->u.constant.envelope; - default: - return &empty_envelope; + case FF_PERIODIC: + return &effect->u.periodic.envelope; + case FF_CONSTANT: + return &effect->u.constant.envelope; + default: + return &empty_envelope; } } diff --git a/drivers/input/joydev.c b/drivers/input/joydev.c index 26043cc..78f323e 100644 --- a/drivers/input/joydev.c +++ b/drivers/input/joydev.c @@ -711,7 +711,7 @@ static long joydev_ioctl(struct file *file, case JS_SET_ALL: retval = copy_from_user(&joydev->glue, argp, - sizeof(joydev->glue)) ? -EFAULT: 0; + sizeof(joydev->glue)) ? -EFAULT : 0; break; case JS_GET_ALL: diff --git a/drivers/input/mousedev.c b/drivers/input/mousedev.c index 0110b5a..80bbe52 100644 --- a/drivers/input/mousedev.c +++ b/drivers/input/mousedev.c @@ -236,25 +236,36 @@ static void mousedev_key_event(struct mousedev *mousedev, case BTN_TOUCH: case BTN_0: - case BTN_LEFT: index = 0; break; + case BTN_LEFT: + index = 0; + break; case BTN_STYLUS: case BTN_1: - case BTN_RIGHT: index = 1; break; + case BTN_RIGHT: + index = 1; + break; case BTN_2: case BTN_FORWARD: case BTN_STYLUS2: - case BTN_MIDDLE: index = 2; break; + case BTN_MIDDLE: + index = 2; + break; case BTN_3: case BTN_BACK: - case BTN_SIDE: index = 3; break; + case BTN_SIDE: + index = 3; + break; case BTN_4: - case BTN_EXTRA: index = 4; break; + case BTN_EXTRA: + index = 4; + break; - default: return; + default: + return; } if (value) { @@ -551,17 +562,16 @@ static int mousedev_open(struct inode *inode, struct file *file) return -ENODEV; error = mutex_lock_interruptible(&mousedev_table_mutex); - if (error) { + if (error) return error; - } + mousedev = mousedev_table[i]; if (mousedev) get_device(&mousedev->dev); mutex_unlock(&mousedev_table_mutex); - if (!mousedev) { + if (!mousedev) return -ENODEV; - } client = kzalloc(sizeof(struct mousedev_client), GFP_KERNEL); if (!client) { @@ -1088,7 +1098,7 @@ static int __init mousedev_init(void) #ifdef CONFIG_INPUT_MOUSEDEV_PSAUX error = misc_register(&psaux_mouse); if (error) - pr_warning("could not register psaux device, error: %d\n", + pr_warn("could not register psaux device, error: %d\n", error); else psaux_registered = 1; diff --git a/drivers/input/sparse-keymap.c b/drivers/input/sparse-keymap.c index 75fb040..a70aa55 100644 --- a/drivers/input/sparse-keymap.c +++ b/drivers/input/sparse-keymap.c @@ -180,11 +180,11 @@ int sparse_keymap_setup(struct input_dev *dev, for (e = keymap; e->type != KE_END; e++) map_size++; - map = kcalloc(map_size, sizeof (struct key_entry), GFP_KERNEL); + map = kcalloc(map_size, sizeof(struct key_entry), GFP_KERNEL); if (!map) return -ENOMEM; - memcpy(map, keymap, map_size * sizeof (struct key_entry)); + memcpy(map, keymap, map_size * sizeof(struct key_entry)); for (i = 0; i < map_size; i++) { entry = &map[i];
Fexed coding style issues from scripts/checkpatch.pl in drivers/input Signed-off-by: Baodong Chen <chenbdchenbd@gmail.com> --- drivers/input/apm-power.c | 2 +- drivers/input/evdev.c | 52 +++++++++++++++++++++++++++++++--------- drivers/input/ff-core.c | 4 +- drivers/input/ff-memless.c | 12 ++++---- drivers/input/joydev.c | 2 +- drivers/input/mousedev.c | 32 ++++++++++++++++-------- drivers/input/sparse-keymap.c | 4 +- 7 files changed, 73 insertions(+), 35 deletions(-)