Received-SPF: pass (google.com: domain of linux-kernel+bounces-35596-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Tue, 23 Jan 2024 07:51:51 -0800
From: Bjorn Andersson <quic_bjorande@quicinc.com>
To: Joel Granados <j.granados@samsung.com>
CC: Huang Yiwei <quic_hyiwei@quicinc.com>, <rostedt@goodmis.org>,
        <mhiramat@kernel.org>, <mark.rutland@arm.com>, <mcgrof@kernel.org>,
        <keescook@chromium.org>, <mathieu.desnoyers@efficios.com>,
        <corbet@lwn.net>, <linux-kernel@vger.kernel.org>,
        <linux-trace-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
        <linux-doc@vger.kernel.org>, <quic_tsoni@quicinc.com>,
        <quic_satyap@quicinc.com>, <quic_aiquny@quicinc.com>,
        <kernel@quicinc.com>, Ross Zwisler
	<zwisler@google.com>,
        Joel Fernandes <joel@joelfernandes.org>
Subject: Re: [PATCH v3] tracing: Support to dump instance traces by
 ftrace_dump_on_oops
Message-ID: <20240123155151.GE2936378@hu-bjorande-lv.qualcomm.com>
References: <CGME20240119080907eucas1p12c357eae722d3a60d82c66b81cfc05ba@eucas1p1.samsung.com>
 <20240119080824.907101-1-quic_hyiwei@quicinc.com>
 <20240122135645.danb777cc5e7i77z@localhost>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240122135645.danb777cc5e7i77z@localhost>

On Mon, Jan 22, 2024 at 02:56:45PM +0100, Joel Granados wrote:
> On Fri, Jan 19, 2024 at 04:08:24PM +0800, Huang Yiwei wrote:
[..]
> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
[..]
> > +enum ftrace_dump_mode get_ftrace_dump_mode(void)
> > +{
> > +	if (!strcmp("0", ftrace_dump_on_oops))
> Would using a strncmp be better in this case? And this question goes for
> all the strcmp in the patch. Something like strncmp("0",
> ftrace_dump_on_oops, 1); when they are equal, it would avoid 2
> assignments and two comparisons.

As you determine yourself below, Huang is looking for the string "0" not
just something with the first character being '0', so you you need to
check for null termination.

> Also might avoid runaway comparisons if
> the first string constant changes in the future.
> 

If the constant suddenly isn't null terminated, causing strcmp to run
"endlessly", we have bigger problems.

> Or maybe strncmp("0", ftrace_dump_on_oops, 2); if you want to check if
> they are both null terminated.
> 

This is just obscure. At best it would confuse future readers.

Regards,
Bjorn