linux-kernel - A style question: repeated return value check

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:	Mon, 29 Sep 2008 22:14:39 +0300
From:	Pekka Paalanen <pq@....fi>
To:	linux-kernel@...r.kernel.org
Cc:	Frederic Weisbecker <fweisbec@...il.com>, mingo@...e.hu,
	rostedt@...dmis.org
Subject: A style question: repeated return value check

On Mon, 29 Sep 2008 20:18:34 +0200
Frederic Weisbecker <fweisbec@...il.com> wrote:

> We need a kind of disambiguation when a print_line callback
> returns 0.
> 
> _There is not enough space to print all the entry. 
>  Please flush the seq and retry.
> _I can't handle this type of entry
> 
> This patch changes the type of this callback for better information.
> 
> Also some changes have been made in this V2.
> 
> _ Only relay to default functions after the print_line callback fails.
> _ This patch doesn't fix the issue with the broken pipe (see patch 2/4 for that)
> 
> Some things are still in discussion:
> 
> _ Find better names for the enum print_line_t values
> _ Change the type of print_trace_line into boolean.
> 
> Patches to change that can be sent later.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> ---
>  kernel/trace/trace.c |   77 ++++++++++++++++++++++++++-----------------------
>  kernel/trace/trace.h |   10 ++++++-
>  2 files changed, 50 insertions(+), 37 deletions(-)
> 
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 6ada059..61f33da 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
[...]
> @@ -1633,24 +1633,24 @@ static int print_trace_fmt(struct trace_iterator *iter)
>  
>  	ret = trace_seq_printf(s, "%16s-%-5d ", comm, field->pid);
>  	if (!ret)
> -		return 0;
> +		return TRACE_TYPE_PARTIAL_LINE;
>  	ret = trace_seq_printf(s, "[%03d] ", iter->cpu);
>  	if (!ret)
> -		return 0;
> +		return TRACE_TYPE_PARTIAL_LINE;
>  	ret = trace_seq_printf(s, "%5lu.%06lu: ", secs, usec_rem);
>  	if (!ret)
> -		return 0;
> +		return TRACE_TYPE_PARTIAL_LINE;

Off-thread style question: Would it be better or worse to write the
above as

	ret = trace_seq_printf(s, "%16s-%-5d ", comm, field->pid);
	ret = ret && trace_seq_printf(s, "[%03d] ", iter->cpu);
	ret = ret && trace_seq_printf(s, "%5lu.%06lu: ", secs, usec_rem);
	if (!ret)
		return TRACE_TYPE_PARTIAL_LINE;

which would do exactly the same, but is more compact.
Good or bad style?

Well, it could be rolled into a single trace_seq_printf() call, too.

-- 
Pekka Paalanen
http://www.iki.fi/pq/
--
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/