From: Jingbo Xu <jefflexu@linux.alibaba.com>
To: Gao Xiang <hsiangkao@linux.alibaba.com>,
Chunhai Guo <guochunhai@vivo.com>,
xiang@kernel.org
Cc: linux-erofs@lists.ozlabs.org, huyue2@coolpad.com
Subject: Re: [PATCH v2] erofs: make erofs_err() and erofs_info() support NULL sb parameter
Date: Wed, 3 Jan 2024 12:13:59 +0800 [thread overview]
Message-ID: <824a3a47-202e-41ee-91b4-413d152b64e1@linux.alibaba.com> (raw)
In-Reply-To: <c792639e-c202-4de8-9218-424044fe4c66@linux.alibaba.com>
On 1/3/24 4:28 AM, Gao Xiang wrote:
> Hi Jingbo,
>
> On 2024/1/2 23:10, Jingbo Xu wrote:
>> Hi Chunhai,
>>
>> Thanks for the commit.
>>
>> On 1/2/24 11:01 PM, Chunhai Guo wrote:
>>> Make erofs_err() and erofs_info() support NULL sb parameter for more
>>> general usage.
>>>
>>> Suggested-by: Gao Xiang <xiang@kernel.org>
>>> Signed-off-by: Chunhai Guo <guochunhai@vivo.com>
>>> ---
>>> fs/erofs/super.c | 10 ++++++++--
>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/erofs/super.c b/fs/erofs/super.c
>>> index 3789d6224513..5f60f163bd56 100644
>>> --- a/fs/erofs/super.c
>>> +++ b/fs/erofs/super.c
>>> @@ -27,7 +27,10 @@ void _erofs_err(struct super_block *sb, const char
>>> *func, const char *fmt, ...)
>>> vaf.fmt = fmt;
>>> vaf.va = &args;
>>> - pr_err("(device %s): %s: %pV", sb->s_id, func, &vaf);
>>> + if (sb)
>>> + pr_err("(device %s): %s: %pV", sb->s_id, func, &vaf);
>>> + else
>>> + pr_err("%s: %pV", func, &vaf);
>>> va_end(args);
>>> }
>>> @@ -41,7 +44,10 @@ void _erofs_info(struct super_block *sb, const
>>> char *func, const char *fmt, ...)
>>> vaf.fmt = fmt;
>>> vaf.va = &args;
>>> - pr_info("(device %s): %pV", sb->s_id, &vaf);
>>> + if (sb)
>>> + pr_info("(device %s): %pV", sb->s_id, &vaf);
>>> + else
>>> + pr_info("%pV", &vaf);
>>> va_end(args);
>>> }
>>>
>>
>> May I ask in which case the input sb parameter will be NULL? In that
>> case, why don't we call pr_err() or pr_info() directly?
>
> That was my suggestion. erofs_err/info are widely used in the codebase
> and they emit the final "\n" but pr_err/pr_info need to use "\n" in
> contrast.
>
> Combining these two usage might not quite good. Does it sound good to
> you?
>
> Also, it is worth converting the only one to erofs_err now.
> decompressor_deflate.c: pr_err("failed to allocate zlib workspace\n");
>
> THNKS,
> GAO XIANG
Thanks. LGTM.
Reviewed-by: Jingbo Xu <jefflexu@linux.alibaba.com>
--
Thanks,
Jingbo
prev parent reply other threads:[~2024-01-03 4:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-02 15:01 [PATCH v2] erofs: make erofs_err() and erofs_info() support NULL sb parameter Chunhai Guo via Linux-erofs
2024-01-02 15:10 ` Jingbo Xu
2024-01-02 20:28 ` Gao Xiang
2024-01-03 4:13 ` Jingbo Xu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=824a3a47-202e-41ee-91b4-413d152b64e1@linux.alibaba.com \
--to=jefflexu@linux.alibaba.com \
--cc=guochunhai@vivo.com \
--cc=hsiangkao@linux.alibaba.com \
--cc=huyue2@coolpad.com \
--cc=linux-erofs@lists.ozlabs.org \
--cc=xiang@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).