[proto] xprint: Fix build of xprint extension

Submitted by Olivier Fourdan on May 19, 2015, 2:04 p.m.

Details

Message ID 1432044251-13991-1-git-send-email-ofourdan@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Olivier Fourdan May 19, 2015, 2:04 p.m.
Generated XCB code based on xprint.xml won't compile because the
arguments do not match now that the accessor functions for requests are
generated in libxcb.

Fix the definition so that the arguments match and the generated C code
for XPrint can compile.

Note, XPrint support has been removed in the X server since 2008, so
this is mostly cosmetic or even pedantic because XPrint is long gone.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
---
 src/xprint.xml | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/xprint.xml b/src/xprint.xml
index dc23dd0..4da49b0 100644
--- a/src/xprint.xml
+++ b/src/xprint.xml
@@ -178,9 +178,13 @@  authorization from the authors.
             <fieldref>len_data</fieldref>
         </list>
         <!-- padding -->
-        <list type="STRING8" name="doc_format" />
+        <list type="STRING8" name="doc_format">
+            <fieldref>len_fmt</fieldref>
+        </list>
         <!-- padding -->
-        <list type="STRING8" name="options" />
+        <list type="STRING8" name="options">
+            <fieldref>len_options</fieldref>
+        </list>
     </request>
 
     <request name="PrintGetDocumentData" opcode="12">

Comments

On 2015-05-19 10:04, Olivier Fourdan wrote:
> Generated XCB code based on xprint.xml won't compile because the
> arguments do not match now that the accessor functions for requests are
> generated in libxcb.
> 
> Fix the definition so that the arguments match and the generated C code
> for XPrint can compile.
> 
> Note, XPrint support has been removed in the X server since 2008, so
> this is mostly cosmetic or even pedantic because XPrint is long gone.
> 
> Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>

The fix is obviously correct, or at least obviously better, because a
<list> with an implicit length is only valid as the last member of a
request.

> ---
>  src/xprint.xml | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/xprint.xml b/src/xprint.xml
> index dc23dd0..4da49b0 100644
> --- a/src/xprint.xml
> +++ b/src/xprint.xml
> @@ -178,9 +178,13 @@ authorization from the authors.
>              <fieldref>len_data</fieldref>
>          </list>
>          <!-- padding -->

Since you're fixing the protocol definition of this request, you could
also change the <!-- padding --> comment to <pad align="4" />. ("pad
align" is relatively recent, and I'm guessing whoever originally wrote
xprint.xml was too lazy to put in the annoyingly large <op> block
required to do an alignment pad without "pad align").

> -        <list type="STRING8" name="doc_format" />
> +        <list type="STRING8" name="doc_format">
> +            <fieldref>len_fmt</fieldref>
> +        </list>
>          <!-- padding -->

Here too.

> -        <list type="STRING8" name="options" />
> +        <list type="STRING8" name="options">
> +            <fieldref>len_options</fieldref>
> +        </list>
>      </request>

That said, this looks good. With or without the <pad align="4" /> changes,
Reviewed-by: Peter Harris <pharris@opentext.com>

Peter Harris
Hi Peter,

> Since you're fixing the protocol definition of this request, you could
> also change the <!-- padding --> comment to <pad align="4" />. ("pad
> align" is relatively recent, and I'm guessing whoever originally wrote
> xprint.xml was too lazy to put in the annoyingly large <op> block
> required to do an alignment pad without "pad align").

If I add the <pad align="4" /> statement, the generated code won't build anymore :(

It fails with:

xprint.c: In function 'xcb_x_print_print_put_document_data_doc_format_end':
xprint.c:1338:36: error: 'None' undeclared (first use in this function)
     xcb_generic_iterator_t child = None;
                                    ^
xprint.c:1338:36: note: each undeclared identifier is reported only once for each function it appears in
xprint.c: In function 'xcb_x_print_print_put_document_data_options_end':
xprint.c:1362:36: error: 'None' undeclared (first use in this function)
     xcb_generic_iterator_t child = None;
                                    ^
Not sure why padding does that...

> That said, this looks good. With or without the <pad align="4" /> changes,
> Reviewed-by: Peter Harris <pharris@opentext.com>

Cheers!
Olivier