Hi Tom,
Thank you for your feedback.
You're absolutely right — the subject and commit message were too generic
and wordy. I've revised the patch accordingly and sent a v2 with the
subject:
[PATCH v2] tiny-printf: handle NULL pointer for %s format string
The v2 includes:
- Updated subject prefix to reflect the correct subsystem (tiny-printf),
- Simplified and clarified the commit message as per your suggestions,
- No code changes — just improved presentation.
Thanks again for the helpful review!
Best regards,
Anton Moryakov
пн, 19 мая 2025 г. в 22:11, Tom Rini <tr...@konsulko.com>:
> On Fri, May 16, 2025 at 09:54:47PM +0300, ant.v.morya...@gmail.com wrote:
>
> > From: Anton Moryakov <ant.v.morya...@gmail.com>
> >
> > Handle NULL pointer case in string formatting (%s) by printing '(null)'
> > instead of dereferencing NULL pointer. Makes behavior consistent with
> > standard printf implementations and prevents potential crashes.
> >
> > Signed-off-by: Anton Moryakov <ant.v.morya...@gmail.com>"
>
> First, this is tiny-printf so the subject should be changed to be
> clearer about the impact. This is only used in very constrained
> environments.
>
> > ---
> > lib/tiny-printf.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> > index faf55d7f327..da596d72050 100644
> > --- a/lib/tiny-printf.c
> > +++ b/lib/tiny-printf.c
> > @@ -306,6 +306,8 @@ static int _vprintf(struct printf_info *info, const
> char *fmt, va_list va)
> > break;
> > case 's':
> > p = va_arg(va, char*);
> > + if (!p)
> > + p = "(null)";
> > break;
> > case '%':
> > out(info, '%');
>
> It's so constrained that I'd rather not include "(null)". We should end
> up printing nothing in this case, as this is handled in commit
> f0dab28915b7 ("tiny-printf: Handle NULL pointer argument to %s") and
> having a print where we just omit something, in this constrained
> environment, is acceptable (and should be the debug hint needed).
>
> --
> Tom
>
