Messages in this thread | | | Subject | Re: [PATCH] cdrom: fixed a brace coding style issue | From | Joe Perches <> | Date | Tue, 27 Aug 2013 14:38:23 -0700 |
| |
On Tue, 2013-08-27 at 22:01 +0100, Imededdine Aimene Bounab wrote: > Fixed a coding style issue.
Hi Imededdine.
Just some simple notes.
Your subject line doesn't match the patch and you are doing more than a single brace change.
There are lots of whitespace changes.
Maybe multiple patches for easier review would be better.
> diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c [] > /* These are used to simplify getting data in from and back to user land */ > #define IOCTL_IN(arg, type, in) \ > - if (copy_from_user(&(in), (type __user *) (arg), sizeof (in))) \ > + if (copy_from_user(&(in), (type __user *) (arg), sizeof(in))) \ > return -EFAULT; > > #define IOCTL_OUT(arg, type, out) \ > - if (copy_to_user((type __user *) (arg), &(out), sizeof (out))) \ > + if (copy_to_user((type __user *) (arg), &(out), sizeof(out))) \ > return -EFAULT;
These might as well be expanded in-place.
> @@ -340,7 +340,7 @@ do { \ > #define CDROM_CAN(type) (cdi->ops->capability & ~cdi->mask & (type)) > > /* used in the audio ioctls */ > -#define CHECKAUDIO if ((ret=check_for_audio_disc(cdi, cdo))) return ret > +#define CHECKAUDIO if ((ret = check_for_audio_disc(cdi, cdo))) return ret
This CHECKAUDIO #define isn't used anywhere and could be deleted.
> @@ -363,7 +363,8 @@ static void cdrom_count_tracks(struct cdrom_device_info *, tracktype*); > > static int cdrom_mrw_exit(struct cdrom_device_info *cdi); > > -static int cdrom_get_disc_info(struct cdrom_device_info *cdi, disc_information *di); > +static int cdrom_get_disc_info(struct cdrom_device_info *cdi, > + disc_information *di);
Please be consistent with with multi-line statement indentation.
If this is really an 80 column problem (and if it is, it's a small one), please use alignment to open paren like:
static int cdrom_get_disc_info(struct cdrom_device_info *cdi, disc_information *di);
(maximal 8 indent tabs, spaces remainder)
etc...
| |