[Xcb] Proposed patch to support clang compilation of util-image

[Pablo Cholaky]

Guys,

I think this patch is currently harmless and makes support to clang to
properly compile it.

Author: Pablo Cholaky <***@slash.cl>
Date: Sat Oct 8 16:52:37 2016 -0300

Added return to xcb_host_byte_order function

Signed-off-by: Pablo Cholaky <***@slash.cl>

diff --git a/image/xcb_bitops.h b/image/xcb_bitops.h
index a6872a1..bf6fdc1 100644
--- a/image/xcb_bitops.h
+++ b/image/xcb_bitops.h
@@ -207,6 +207,7 @@ xcb_host_byte_order(void) {
return XCB_IMAGE_ORDER_LSB_FIRST;
}
assert(0);
+ return -1;
}

#endif /* __XCB_BITOPS_H__ */

Using clang it fails just after configure

Making all in image
make[2]: Entering directory
'/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0-abi_x86_64.amd64/image'
/bin/sh ../libtool --tag=CC --mode=compile clang -DHAVE_CONFIG_H -I.
-I/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image
-I.. -Wall -Wpointer-arith -Wmissing-declarations -Wformat=2
-Wstrict-prototypes -Wmissing-prototypes -Wnested-externs
-Wbad-function-cast -Wold-style-definition -Wdeclaration-after-statement
-Wunused -Wuninitialized -Wshadow -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Werror=implicit
-Werror=nonnull -Werror=init-self -Werror=main -Werror=missing-braces
-Werror=sequence-point -Werror=return-type -Werror=trigraphs
-Werror=array-bounds -Werror=write-strings -Werror=address
-Werror=int-to-pointer-cast -Werror=pointer-to-int-cast
-fno-strict-aliasing -O2 -pipe -g -c -o xcb_image.lo
/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image/xcb_image.c
libtool: compile: clang -DHAVE_CONFIG_H -I.
-I/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image
-I.. -Wall -Wpointer-arith -Wmissing-declarations -Wformat=2
-Wstrict-prototypes -Wmissing-prototypes -Wnested-externs
-Wbad-function-cast -Wold-style-definition -Wdeclaration-after-statement
-Wunused -Wuninitialized -Wshadow -Wmissing-noreturn
-Wmissing-format-attribute -Wredundant-decls -Werror=implicit
-Werror=nonnull -Werror=init-self -Werror=main -Werror=missing-braces
-Werror=sequence-point -Werror=return-type -Werror=trigraphs
-Werror=array-bounds -Werror=write-strings -Werror=address
-Werror=int-to-pointer-cast -Werror=pointer-to-int-cast
-fno-strict-aliasing -O2 -pipe -g -c
/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image/xcb_image.c
-fPIC -DPIC -o .libs/xcb_image.o
In file included from
/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image/xcb_image.c:37:
/var/tmp/portage/x11-libs/xcb-util-image-0.4.0/work/xcb-util-image-0.4.0/image/xcb_bitops.h:210:1:
error: control may reach end of non-void function [-Werror,-Wreturn-type]
}
^
1 error generated.

Based on the patch initially proposed here:
https://patchwork.openembedded.org/patch/118073/ and discussed here
https://github.com/gentoo/gentoo/pull/2496 and here
https://github.com/gentoo/musl/pull/12

Regards.

[Alan Coopersmith]

I think this patch is currently harmless and makes support to clang to properly
compile it.

clang doesn't complain on the current code for me (probably because my libc
headers define the function called on a failed assert with the noreturn

assert(0);
+ return -1;

causes clang to report two new warnings for me:

./xcb_bitops.h:210:10: warning: integer constant not in range of
enumerated type 'xcb_image_order_t' (aka 'enum xcb_image_order_t')
[-Wassign-enum]
return -1;
^
./xcb_bitops.h:210:10: warning: implicit conversion changes signedness:
'int' to 'xcb_image_order_t' (aka 'enum xcb_image_order_t')
[-Wsign-conversion]
return -1;
~~~~~~ ^~

Admittedly I run clang with more warnings enabled than the default build, to
help find issues during development.

[Bart Massey]

Yeah, assert() should be marked noreturn (directly or indirectly) in
general or bad things happen. Those assert()s really want to be assert()s
or calls to abort() or something: there is nothing good that can happen
after returning from those places.

On Sun, Oct 16, 2016 at 11:19 PM Alan Coopersmith <

Post by Pablo Cholaky

Post by Pablo Cholaky
I think this patch is currently harmless and makes support to clang to

properly

Post by Pablo Cholaky
compile it.

clang doesn't complain on the current code for me (probably because my libc
headers define the function called on a failed assert with the noreturn

Post by Pablo Cholaky
assert(0);
+ return -1;

./xcb_bitops.h:210:10: warning: integer constant not in range of
enumerated type 'xcb_image_order_t' (aka 'enum xcb_image_order_t')
[-Wassign-enum]
return -1;
^
'int' to 'xcb_image_order_t' (aka 'enum xcb_image_order_t')
[-Wsign-conversion]
return -1;
~~~~~~ ^~
Admittedly I run clang with more warnings enabled than the default build, to
help find issues during development.
--
Oracle Solaris Engineering - http://blogs.oracle.com/alanc
_______________________________________________
Xcb mailing list
https://lists.freedesktop.org/mailman/listinfo/xcb

[Josh Triplett]

Post by Bart Massey
Yeah, assert() should be marked noreturn (directly or indirectly) in
general or bad things happen. Those assert()s really want to be assert()s
or calls to abort() or something: there is nothing good that can happen
after returning from those places.

I just checked, and clang definitely does support the noreturn
attribute. And the system headers typically define assert with the
noreturn attribute.

Did the build occur with NDEBUG defined or similar, to no-op out the
assert?

[Pablo Cholaky]

Post by Josh Triplett

Post by Bart Massey
Yeah, assert() should be marked noreturn (directly or indirectly) in
general or bad things happen. Those assert()s really want to be assert()s
or calls to abort() or something: there is nothing good that can happen
after returning from those places.

I just checked, and clang definitely does support the noreturn
attribute. And the system headers typically define assert with the
noreturn attribute.
Did the build occur with NDEBUG defined or similar, to no-op out the
assert?

Guys, I'm really sorry of being late responding this, the problem was
caused by musl and not clang nor xcb-image, so please ignore those changes.

Reference:
http://git.musl-libc.org/cgit/musl/commit/?id=e738b8cbe64b6dd3ed9f47b6d4cd7eb2c422b38d

Regards!