Re: [PATCH 2/3] checks: Make each message output atomic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Thu, Nov 22, 2018 at 7:03 PM David Gibson
<david@xxxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Thu, Nov 22, 2018 at 12:45:20PM -0600, Rob Herring wrote:
> > On Thu, Nov 22, 2018 at 5:16 AM David Gibson
> > <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Wed, Nov 21, 2018 at 03:53:58PM -0600, Rob Herring wrote:
> > > > Printing to stderr as we build up the check message results in
> > > > interleaving of messages when multiple instances of dtc are running.
> > > > Change the message output to use an intermediate buffer for constructing
> > > > the message and then output the message to stderr with a single fputs.
> > > >
> > > > While perhaps there is no guarantee that fputs will be atomic, this gets
> > > > rid of any interleaved output that previously occurred on Linux.
> > > >
> > > > Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> > >
> > > I can see that this would be beneficial, but..
> > >
> > > > ---
> > > >  checks.c | 29 +++++++++++++++++++++--------
> > > >  1 file changed, 21 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/checks.c b/checks.c
> > > > index 4d8dffd36b6e..e60940d2af86 100644
> > > > --- a/checks.c
> > > > +++ b/checks.c
> > > > @@ -72,29 +72,42 @@ struct check {
> > > >  #define CHECK(nm_, fn_, d_, ...) \
> > > >       CHECK_ENTRY(nm_, fn_, d_, false, false, __VA_ARGS__)
> > > >
> > > > +#define MAX_MSG_LEN 1024
> > >
> > > I really hate arbitrary buffer limits.
> > >
> > > We are already have an xasprintf() function, so lets use that.
> >
> > Okay, that's what I wrote first...
> >
> > It gets kind of ugly because you end up with a string for each segment
> > and then assembled at the end. This is really problematic for the
> > "also defined at ..." strings as we don't know how many we have. So
> > count how many we have, allocate an array of char* ptrs, and then loop
> > again?
>
> Ok, sounds like you want an xasprintf_append() helper,
>         new = xasprintf("%s" fmt, old, ...)
>         free(old)
>         return new

Reality is a bit more complicated since you can't call a variable args
to another variadic function and have to use va_list. The result is
xasprintf is a helper calling xavsprintf_append.

Rob



[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux