Re: [PATCH 4/4] kernel:audit.c fixed a coding style issue

[Eric Paris]

On Tue, 2017-02-28 at 21:49 +0000, Joan Jani wrote:
> This patch fixes the following checkpath.pl warning
> ÂWARNING: Block comments use a trailing */ on a separate line
> 
> like
> 
> kernel/audit.c:135: WARNING: Block comments use a trailing */ on a
> separate line
> kernel/audit.c:170: WARNING: Block comments use a trailing */ on a
> separate line
> kernel/audit.c:174: WARNING: Block comments use a trailing */ on a
> separate line
> kernel/audit.c:181: WARNING: Block comments use a trailing */ on a
> 
> and some more style. No changes to code
> 
> Signed-off-by: Joan Jani <igiann@xxxxxxxxxxx>
> ---
> Âkernel/audit.c | 53 ++++++++++++++++++++++++++++++++++------------
> -------
> Â1 file changed, 34 insertions(+), 19 deletions(-)
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index e794544..62d90d9 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -70,7 +70,8 @@
> Â#include "audit.h"
> Â
> Â/* No auditing will take place until audit_initialized ==
> AUDIT_INITIALIZED.
> - * (Initialization happens after skb_init is called.) */
> + * (Initialization happens after skb_init is called.)
> + */
> Â#define AUDIT_DISABLED		-1
> Â#define AUDIT_UNINITIALIZED	0
> Â#define AUDIT_INITIALIZED	1
> @@ -100,11 +101,13 @@ static __u32	audit_nlk_portid;
> Â
> Â/* If audit_rate_limit is non-zero, limit the rate of sending audit
> records
> Â * to that number per second.ÂÂThis prevents DoS attacks, but
> results in
> - * audit records being dropped. */
> + * audit records being dropped.
> + */

After Linus's polite request around comment style I think I'm going to
have to disagree with these parts of the patch...

http://lkml.iu.edu/hypermail/linux/kernel/1607.1/00627.html




> Âstatic u32	audit_rate_limit;
> Â
> Â/* Number of outstanding audit_buffers allowed.
> - * When set to zero, this means unlimited. */
> + * When set to zero, this means unlimited.
> + */
> Âstatic u32	audit_backlog_limit = 64;
> Â#define AUDIT_BACKLOG_WAIT_TIME (60 * HZ)
> Âstatic u32	audit_backlog_wait_time = AUDIT_BACKLOG_WAIT_TIME;
> @@ -115,7 +118,7 @@ pid_t		audit_sig_pid = -1;
> Âu32		audit_sig_sid = 0;
> Â
> Â/* Records can be lost in several ways:
> -ÂÂÂ0) [suppressed in audit_alloc]
> + * 0) [suppressed in audit_alloc]
> ÂÂÂÂ1) out of memory in audit_log_start [kmalloc of struct
> audit_buffer]
> ÂÂÂÂ2) out of memory in audit_log_move [alloc_skb]
> ÂÂÂÂ3) suppressed due to audit_rate_limit
> @@ -132,7 +135,8 @@ struct list_head
> audit_inode_hash[AUDIT_INODE_BUCKETS];
> Â
> Â/* The audit_freelist is a list of pre-allocated audit buffers (if
> more
> Â * than AUDIT_MAXFREE are in use, the audit buffer is freed instead
> of
> - * being placed on the freelist). */
> + * being placed on the freelist).
> + */
> Âstatic DEFINE_SPINLOCK(audit_freelist_lock);
> Âstatic int	ÂÂÂaudit_freelist_count;
> Âstatic LIST_HEAD(audit_freelist);
> @@ -167,18 +171,21 @@ DEFINE_MUTEX(audit_cmd_mutex);
> Â
> Â/* AUDIT_BUFSIZ is the size of the temporary buffer used for
> formatting
> Â * audit records.ÂÂSince printk uses a 1024 byte buffer, this buffer
> - * should be at least that large. */
> + * should be at least that large.
> + */
> Â#define AUDIT_BUFSIZ 1024
> Â
> Â/* AUDIT_MAXFREE is the number of empty audit_buffers we keep on the
> - * audit_freelist.ÂÂDoing so eliminates many kmalloc/kfree calls. */
> + * audit_freelist.ÂÂDoing so eliminates many kmalloc/kfree calls.
> + */
> Â#define AUDIT_MAXFREEÂÂ(2*NR_CPUS)
> Â
> Â/* The audit_buffer is used when formatting an audit record.ÂÂThe
> caller
> Â * locks briefly to get the record off the freelist or to allocate
> the
> Â * buffer, and locks briefly to send the buffer to the netlink layer
> or
> Â * to place it on a transmit queue.ÂÂMultiple audit_buffers can be
> in
> - * use simultaneously. */
> + * use simultaneously.
> + */
> Âstruct audit_buffer {
> Â	struct list_headÂÂÂÂÂlist;
> Â	struct sk_buffÂÂÂÂÂÂÂ*skb;	/* formatted skb ready to
> send */
> @@ -227,7 +234,8 @@ static inline int audit_rate_check(void)
> Â	unsigned long		elapsed;
> Â	int			retval	ÂÂÂ= 0;
> Â
> -	if (!audit_rate_limit) return 1;
> +	if (!audit_rate_limit)
> +		return 1;
> Â
> Â	spin_lock_irqsave(&lock, flags);
> Â	if (++messages < audit_rate_limit) {
> @@ -253,7 +261,7 @@ static inline int audit_rate_check(void)
> Â * Emit at least 1 message per second, even if audit_rate_check is
> Â * throttling.
> Â * Always increment the lost messages counter.
> -*/
> + */
> Âvoid audit_log_lost(const char *message)
> Â{
> Â	static unsigned long	last_msg = 0;
> @@ -350,6 +358,7 @@ static int audit_set_backlog_wait_time(u32
> timeout)
> Âstatic int audit_set_enabled(u32 state)
> Â{
> Â	int rc;
> +
> Â	if (state > AUDIT_LOCKED)
> Â		return -EINVAL;
> Â
> @@ -402,7 +411,8 @@ static void kauditd_printk_skb(struct sk_buff
> *skb)
> Âstatic void kauditd_hold_skb(struct sk_buff *skb)
> Â{
> Â	/* at this point it is uncertain if we will ever send this
> to auditd so
> -	Â* try to send the message via printk before we go any
> further */
> +	Â* try to send the message via printk before we go any
> further
> +	Â*/
> Â	kauditd_printk_skb(skb);
> Â
> Â	/* can we just silently drop the message? */
> @@ -436,7 +446,8 @@ static void kauditd_retry_skb(struct sk_buff
> *skb)
> Â{
> Â	/* NOTE: because records should only live in the retry queue
> for a
> Â	Â* short period of time, before either being sent or moved
> to the hold
> -	Â* queue, we don't currently enforce a limit on this queue
> */
> +	Â* queue, we don't currently enforce a limit on this queue
> +	Â*/
> Â	skb_queue_tail(&audit_retry_queue, skb);
> Â}
> Â
> @@ -555,7 +566,7 @@ static int kauditd_thread(void *dummy)
> Â	struct nlmsghdr *nlh;
> Â
> Â#define UNICAST_RETRIES 5
> -#define AUDITD_BAD(x,y) \
> +#define AUDITD_BAD(x, y) \
> Â	((x) == -ECONNREFUSED || (x) == -EPERM || ++(y) >=
> UNICAST_RETRIES)
> Â
> Â	/* NOTE: we do invalidate the auditd connection flag on any
> sending
> @@ -729,8 +740,9 @@ static int audit_send_reply_thread(void *arg)
> Â	mutex_unlock(&audit_cmd_mutex);
> Â
> Â	/* Ignore failure. It'll only happen if the sender goes
> away,
> -	ÂÂÂbecause our timeout is set to infinite. */
> -	netlink_unicast(aunet->nlsk , reply->skb, reply->portid, 0);
> +	Â* because our timeout is set to infinite.
> +	Â*/
> +	netlink_unicast(aunet->nlsk, reply->skb, reply->portid, 0);
> Â	put_net(net);
> Â	kfree(reply);
> Â	return 0;
> @@ -1311,6 +1323,7 @@ static void __net_exit audit_net_exit(struct
> net *net)
> Â{
> Â	struct audit_net *aunet = net_generic(net, audit_net_id);
> Â	struct sock *sock = aunet->nlsk;
> +
> Â	mutex_lock(&audit_cmd_mutex);
> Â	if (sock == audit_sock)
> Â		auditd_reset();
> @@ -1378,7 +1391,8 @@ static int __init audit_enable(char *str)
> Â__setup("audit=", audit_enable);
> Â
> Â/* Process kernel command-line parameter at boot time.
> - * audit_backlog_limit=<n> */
> + * audit_backlog_limit=<n>Â
> + */
> Âstatic int __init audit_backlog_limit_set(char *str)
> Â{
> Â	u32 audit_backlog_limit_arg;
> @@ -1415,7 +1429,7 @@ static void audit_buffer_free(struct
> audit_buffer *ab)
> Â	spin_unlock_irqrestore(&audit_freelist_lock, flags);
> Â}
> Â
> -static struct audit_buffer * audit_buffer_alloc(struct audit_context
> *ctx,
> +static struct audit_buffer *audit_buffer_alloc(struct audit_context
> *ctx,
> Â						gfp_t gfp_mask, int
> type)
> Â{
> Â	unsigned long flags;
> @@ -1921,7 +1935,8 @@ void audit_log_name(struct audit_context
> *context, struct audit_names *n,
> Â			break;
> Â		case 0:
> Â			/* name was specified as a relative path and
> the
> -			Â* directory component is the cwd */
> +			Â* directory component is the cwd
> +			Â*/
> Â			audit_log_d_path(ab, " name=", &context-
> >pwd);
> Â			break;
> Â		default:
> @@ -1961,7 +1976,7 @@ void audit_log_name(struct audit_context
> *context, struct audit_names *n,
> Â
> Â	/* log the audit_names record type */
> Â	audit_log_format(ab, " nametype=");
> -	switch(n->type) {
> +	switch (n->type) {
> Â	case AUDIT_TYPE_NORMAL:
> Â		audit_log_format(ab, "NORMAL");
> Â		break;