[PATCH xts 2/3] xts5: Fix missing variable for format specifier

[Rhys Kidd]

On 29 October 2016 at 16:45, walter harms <wharms at bfs.de> wrote:

>
>
> Am 29.10.2016 05:37, schrieb Rhys Kidd:
> > XtRemoveEventHandler.c:458:3: warning: format '%d' expects a matching
> 'int' argument [-Wformat=]
> >    sprintf(ebuf, "ERROR: Error message handler was invoked %d times");
> >    ^
> > XtRemoveEventHandler.c:463:3: warning: format '%d' expects a matching
> 'int' argument [-Wformat=]
> >    sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");
> >    ^
> > XtRemoveEventHandler.c:540:3: warning: format '%d' expects a matching
> 'int' argument [-Wformat=]
> >    sprintf(ebuf, "ERROR: Error message handler was invoked %d times");
> >    ^
> > XtRemoveEventHandler.c:545:3: warning: format '%d' expects a matching
> 'int' argument [-Wformat=]
> >    sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");
> >    ^
> >
> > Signed-off-by: Rhys Kidd <rhyskidd at gmail.com>
> > ---
> >  xts5/Xt9/XtRemoveEventHandler.m | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/xts5/Xt9/XtRemoveEventHandler.m b/xts5/Xt9/
> XtRemoveEventHandler.m
> > index 1851ef9..6e6cbbe 100644
> > --- a/xts5/Xt9/XtRemoveEventHandler.m
> > +++ b/xts5/Xt9/XtRemoveEventHandler.m
> > @@ -425,13 +425,13 @@ int invoked = 0;
> >       XtAppMainLoop(app_ctext);
> >       LKROF(pid2, AVSXTTIMEOUT-2);
> >       tet_infoline("TEST: Error message was not emitted");
> > -     if (avs_get_event(3) != 0) {
> > -             sprintf(ebuf, "ERROR: Error message handler was invoked %d
> times");
> > +     if (invoked = avs_get_event(3) != 0) {
> > +             sprintf(ebuf, "ERROR: Error message handler was invoked %d
> times", invoked);
> >               tet_infoline(ebuf);
> >               tet_result(TET_FAIL);
> >       }
>
> IMHO this is to complicated. what is wrong with:
>
> invoked = avs_get_event(3);
> if (invoked != 0) {
>                 sprintf(ebuf, "ERROR: Error message handler was invoked %d
> times", invoked);
>                 tet_infoline(ebuf);
>                 tet_result(TET_FAIL);
>  }
>
>
>
Hello Walter, thanks for your review.

As an early patch from me, I preferred to keep consistent with the pattern
adopted in this part of the code base -- rather than changing the pattern
via refactoring at the same time as making the bug fix.

e.g. you can see another example here: xts5/XtC/XtSetSelectionTimeout.m

I'd prefer to keep this patch as is in this respect. There's nothing
stopping a follow-up patch making your suggested changes.


>
> > -     if (avs_get_event(2) != 0) {
> > -             sprintf(ebuf, "ERROR: Warning message handler was invoked
> %d times");
> > +     if (invoked = avs_get_event(2) != 0) {
> > +             sprintf(ebuf, "ERROR: Warning message handler was invoked
> %d times", invoked);
> >               tet_infoline(ebuf);
> >               tet_result(TET_FAIL);
> >       }
>
>
> btw: this is the same msg als with avs_get_event(3). I have no clue what
> the differenz is
>      but maybe it should be reflected in the errmsg.
>
>
There's a subtle difference.

- The errmsg associated with avs_get_event(2) includes the text "*Warning*
message handler...."
- The errmsg associated with avs_get_event(3) includes the text "*Error*
message handler..."

If there is a problem with the existing approach, which is a pattern
throughout this area of the code, then I'd prefer it is addressed in a
subsequent patch.

Accordingly, can I seek your Reviewed-by?

Regards,
Rhys


> just my 2 cents,
>
> re,
>  wh
>
>
> > @@ -491,13 +491,13 @@ int invoked = 0;
> >               tet_result(TET_FAIL);
> >       }
> >       tet_infoline("TEST: Error message was not emitted");
> > -     if (avs_get_event(3) != 0) {
> > -             sprintf(ebuf, "ERROR: Error message handler was invoked %d
> times");
> > +     if (invoked = avs_get_event(3) != 0) {
> > +             sprintf(ebuf, "ERROR: Error message handler was invoked %d
> times", invoked);
> >               tet_infoline(ebuf);
> >               tet_result(TET_FAIL);
> >       }
> > -     if (avs_get_event(2) != 0) {
> > -             sprintf(ebuf, "ERROR: Warning message handler was invoked
> %d times");
> > +     if (invoked = avs_get_event(2) != 0) {
> > +             sprintf(ebuf, "ERROR: Warning message handler was invoked
> %d times", invoked);
> >               tet_infoline(ebuf);
> >               tet_result(TET_FAIL);
> >       }
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20161029/721e4c5d/attachment.html>