<div dir="ltr">On 29 October 2016 at 16:45, walter harms <span dir="ltr"><<a href="mailto:wharms@bfs.de" target="_blank">wharms@bfs.de</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail-HOEnZb"><div class="gmail-h5"><br>
<br>
Am 29.10.2016 05:37, schrieb Rhys Kidd:<br>
> XtRemoveEventHandler.c:458:3: warning: format '%d' expects a matching 'int' argument [-Wformat=]<br>
>    sprintf(ebuf, "ERROR: Error message handler was invoked %d times");<br>
>    ^<br>
> XtRemoveEventHandler.c:463:3: warning: format '%d' expects a matching 'int' argument [-Wformat=]<br>
>    sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");<br>
>    ^<br>
> XtRemoveEventHandler.c:540:3: warning: format '%d' expects a matching 'int' argument [-Wformat=]<br>
>    sprintf(ebuf, "ERROR: Error message handler was invoked %d times");<br>
>    ^<br>
> XtRemoveEventHandler.c:545:3: warning: format '%d' expects a matching 'int' argument [-Wformat=]<br>
>    sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");<br>
>    ^<br>
><br>
> Signed-off-by: Rhys Kidd <<a href="mailto:rhyskidd@gmail.com">rhyskidd@gmail.com</a>><br>
> ---<br>
>  xts5/Xt9/XtRemoveEventHandler.<wbr>m | 16 ++++++++--------<br>
>  1 file changed, 8 insertions(+), 8 deletions(-)<br>
><br>
> diff --git a/xts5/Xt9/<wbr>XtRemoveEventHandler.m b/xts5/Xt9/<wbr>XtRemoveEventHandler.m<br>
> index 1851ef9..6e6cbbe 100644<br>
> --- a/xts5/Xt9/<wbr>XtRemoveEventHandler.m<br>
> +++ b/xts5/Xt9/<wbr>XtRemoveEventHandler.m<br>
> @@ -425,13 +425,13 @@ int invoked = 0;<br>
>       XtAppMainLoop(app_ctext);<br>
>       LKROF(pid2, AVSXTTIMEOUT-2);<br>
>       tet_infoline("TEST: Error message was not emitted");<br>
> -     if (avs_get_event(3) != 0) {<br>
> -             sprintf(ebuf, "ERROR: Error message handler was invoked %d times");<br>
> +     if (invoked = avs_get_event(3) != 0) {<br>
> +             sprintf(ebuf, "ERROR: Error message handler was invoked %d times", invoked);<br>
>               tet_infoline(ebuf);<br>
>               tet_result(TET_FAIL);<br>
>       }<br>
<br>
</div></div>IMHO this is to complicated. what is wrong with:<br>
<br>
invoked = avs_get_event(3);<br>
if (invoked != 0) {<br>
<span class="gmail-">                sprintf(ebuf, "ERROR: Error message handler was invoked %d times", invoked);<br>
                tet_infoline(ebuf);<br>
                tet_result(TET_FAIL);<br>
 }<br>
<br>
<br></span></blockquote><div><br></div><div>Hello Walter, thanks for your review.<br><br></div><div>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.<br><br></div><div>e.g. you can see another example here: xts5/XtC/XtSetSelectionTimeout.m<br></div><div><br></div><div>I'd prefer to keep this patch as is in this respect. There's nothing stopping a follow-up patch making your suggested changes.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">
<br>
> -     if (avs_get_event(2) != 0) {<br>
> -             sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");<br>
> +     if (invoked = avs_get_event(2) != 0) {<br>
> +             sprintf(ebuf, "ERROR: Warning message handler was invoked %d times", invoked);<br>
>               tet_infoline(ebuf);<br>
>               tet_result(TET_FAIL);<br>
>       }<br>
<br>
<br>
</span>btw: this is the same msg als with avs_get_event(3). I have no clue what the differenz is<br>
     but maybe it should be reflected in the errmsg.<br>
<br></blockquote><div><br></div><div>There's a subtle difference.<br><br></div><div>- The errmsg associated with avs_get_event(2) includes the text "*Warning* message handler...."<br></div><div>- The errmsg associated with avs_get_event(3) includes the text "*Error* message handler..."<br><br></div><div>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.<br><br></div><div>Accordingly, can I seek your Reviewed-by?<br><br></div><div>Regards,<br></div><div>Rhys<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
just my 2 cents,<br>
<br>
re,<br>
 wh<br>
<span class="gmail-"><br>
<br>
> @@ -491,13 +491,13 @@ int invoked = 0;<br>
>               tet_result(TET_FAIL);<br>
>       }<br>
>       tet_infoline("TEST: Error message was not emitted");<br>
> -     if (avs_get_event(3) != 0) {<br>
> -             sprintf(ebuf, "ERROR: Error message handler was invoked %d times");<br>
> +     if (invoked = avs_get_event(3) != 0) {<br>
> +             sprintf(ebuf, "ERROR: Error message handler was invoked %d times", invoked);<br>
>               tet_infoline(ebuf);<br>
>               tet_result(TET_FAIL);<br>
>       }<br>
> -     if (avs_get_event(2) != 0) {<br>
> -             sprintf(ebuf, "ERROR: Warning message handler was invoked %d times");<br>
> +     if (invoked = avs_get_event(2) != 0) {<br>
> +             sprintf(ebuf, "ERROR: Warning message handler was invoked %d times", invoked);<br>
>               tet_infoline(ebuf);<br>
>               tet_result(TET_FAIL);<br>
>       }<br>
</span>______________________________<wbr>_________________<br>
<a href="mailto:xorg-devel@lists.x.org">xorg-devel@lists.x.org</a>: X.Org development<br>
Archives: <a href="http://lists.x.org/archives/xorg-devel" rel="noreferrer" target="_blank">http://lists.x.org/archives/<wbr>xorg-devel</a><br>
Info: <a href="https://lists.x.org/mailman/listinfo/xorg-devel" rel="noreferrer" target="_blank">https://lists.x.org/mailman/<wbr>listinfo/xorg-devel</a></blockquote></div><br></div></div>