linux-kernel - Re: [PATCH 1/4] staging: lustre: fix coding style issues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 9 Sep 2014 01:21:55 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Hugues <morisset.hugues@...il.com>
Cc:	gregkh@...uxfoundation.org, devel@...verdev.osuosl.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] staging: lustre: fix coding style issues

Fix your email "From" header to say your full name instead of just
From: Hugues <morisset.hugues@...il.com>

All the subjects are the same so redo these patches.  Also don't pick
the most vague subject you can think of.  Mention "long lines", for
example.

On Tue, Sep 09, 2014 at 12:04:43AM +0200, Hugues wrote:
> Fix coding style issues:
> * limit the length of changed lines to 80 columns.

Send the patch inline and not as an attachment.

Most of these changes are improvements, but I have some additional
comments inline.

> --- a/drivers/staging/lustre/lustre/include/obd.h
> +++ b/drivers/staging/lustre/lustre/include/obd.h
> @@ -101,7 +101,8 @@ struct lov_stripe_md {
>  		__u32 lw_magic;
>  		__u32 lw_stripe_size;      /* size of the stripe */
>  		__u32 lw_pattern;	  /* striping pattern (RAID0, RAID1) */
> -		__u16 lw_stripe_count;  /* number of objects being striped over */
> +		__u16 lw_stripe_count;
> +			/* number of objects being striped over */

The comment goes on the line before the data so now it looks like it
goes with lw_layout_gen.

>  		__u16 lw_layout_gen;       /* generation of the layout */
>  		char  lw_pool_name[LOV_MAXPOOLNAME]; /* pool name */
> @@ -639,7 +640,8 @@ struct niobuf_local {
>  #define LUSTRE_LWP_NAME		"lwp"
>  
>  /* obd device type names */
> - /* FIXME all the references to LUSTRE_MDS_NAME should be swapped with LUSTRE_MDT_NAME */
> + /* FIXME all the references to LUSTRE_MDS_NAME
> +    should be swapped with LUSTRE_MDT_NAME */

This comment is not in kernel style.  Read Documentation/CodingStyle

> @@ -733,6 +735,7 @@ static inline void oti_init(struct obd_trans_info *oti,
>  	if (req->rq_reqmsg != NULL &&
>  	    lustre_msg_get_flags(req->rq_reqmsg) & MSG_REPLAY) {
>  		__u64 *pre_version = lustre_msg_get_versions(req->rq_reqmsg);
> +
>  		oti->oti_pre_version = pre_version ? pre_version[0] : 0;

This is unrelated to long lines.

>  		oti->oti_transno = lustre_msg_get_transno(req->rq_reqmsg);
>  	}
> @@ -847,14 +850,15 @@ struct obd_device {
>  		      obd_recovering:1,    /* there are recoverable clients */
>  		      obd_abort_recovery:1,/* recovery expired */
>  		      obd_version_recov:1, /* obd uses version checking */
> -		      obd_replayable:1,    /* recovery is enabled; inform clients */
> -		      obd_no_transno:1,    /* no committed-transno notification */
> +		      obd_replayable:1,    /* recovery is enabled;
> +					      inform clients */
> +		      obd_no_transno:1,	/* no committed-transno notification */

Sometimes the code looks better if you just go past the 80 character
limit.  The columns line up nicer.  Remember, checkpatch.pl serves you;
you are not the servant to it.

> @@ -909,9 +913,9 @@ struct obd_device {
>  	int			      obd_requests_queued_for_recovery;
>  	wait_queue_head_t		      obd_next_transno_waitq;
>  	/* protected by obd_recovery_task_lock */
> -	struct timer_list	      obd_recovery_timer;
> -	time_t			   obd_recovery_start; /* seconds */
> -	time_t			   obd_recovery_end; /* seconds, for lprocfs_status */
> +	struct timer_list	obd_recovery_timer;
> +	time_t			obd_recovery_start; /* seconds */
> +	time_t		obd_recovery_end; /* seconds, for lprocfs_status */

Random alignment.

regards,
dan carpenter


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/