Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On 12/18/2013 02:35 AM, Martin Kletzander wrote: When test-locking is enabled, CFLAGS have -Dbool=char, which makes Huh? bool should never be defined to char in CFLAGS; it should only be replaced if gnulib thinks the compiler is too old. pipefd[bool] fail (obviously). Forcing the subscript to be bool by double negation fixes the build breaker. I don't get how this could possibly make a difference. The 'output' variable is declared bool, and then only ever assigned 'false' or 'true', which if bool is replaced by gnulib are still guaranteed to be 0 and 1, so using it directly as an array index is still safe. Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: I still can't build with '--enable-test-locking' and I'm not sure this is the proper way to fix it. Also, the code wasn't touched for months, so I'm not pushing it as a build-breaker. NACK. I need more details about the actual failure you are seeing, but this is not the right patch. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On 12/18/2013 06:09 AM, Eric Blake wrote: On 12/18/2013 02:35 AM, Martin Kletzander wrote: When test-locking is enabled, CFLAGS have -Dbool=char, which makes Huh? bool should never be defined to char in CFLAGS; it should only be replaced if gnulib thinks the compiler is too old. I actually looked at configure.ac, and wow, we really ARE doing -Dbool=char when trying to use CIL. Yuck. I think --with-test-locking has bit-rotted for some time now, and is going to need a LOT of work before it can be considered usable. That option was added in commit 6962a2d, in 2009, which predates my time on libvirt, and I don't ever recall using it myself. I'm not familiar enough with CIL to speculate why it doesn't work with C99 bool (or maybe that was a limitation back in 2009 but has since been fixed). Dan? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On Wed, Dec 18, 2013 at 06:09:40AM -0700, Eric Blake wrote:
On 12/18/2013 02:35 AM, Martin Kletzander wrote:
When test-locking is enabled, CFLAGS have -Dbool=char, which makes
Huh? bool should never be defined to char in CFLAGS; it should only be
replaced if gnulib thinks the compiler is too old.
Gnulib doesn't do that, it's our configure.ac thing (but I must say I
don't know whether that's copied from somewhere or not) introduced in
6962a2dd7e:
AC_ARG_ENABLE([test-locking],
[AS_HELP_STRING([--enable-test-locking],
[thread locking tests using CIL @:@default=no@:@])],
[case ${enableval} in
yes|no) ;;
*) AC_MSG_ERROR([bad value ${enableval} for test-locking option]) ;;
esac],
[enableval=no])
enable_locking=$enableval
if test $enable_locking = yes; then
LOCK_CHECKING_CFLAGS=-Dbool=char -D_Bool=char -save-temps
AC_SUBST([LOCK_CHECKING_CFLAGS])
fi
AM_CONDITIONAL([WITH_CIL],[test $enable_locking = yes])
pipefd[bool] fail (obviously). Forcing the subscript to be bool by
double negation fixes the build breaker.
I don't get how this could possibly make a difference. The 'output'
variable is declared bool, and then only ever assigned 'false' or
'true', which if bool is replaced by gnulib are still guaranteed to be 0
and 1, so using it directly as an array index is still safe.
But the thing is that due to -Dbool=char the variable is declared
char, not bool.
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
I still can't build with '--enable-test-locking' and I'm not sure this
is the proper way to fix it. Also, the code wasn't touched for
months, so I'm not pushing it as a build-breaker.
NACK. I need more details about the actual failure you are seeing, but
this is not the right patch.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On 12/18/2013 06:25 AM, Martin Kletzander wrote: pipefd[bool] fail (obviously). Forcing the subscript to be bool by double negation fixes the build breaker. I don't get how this could possibly make a difference. The 'output' variable is declared bool, and then only ever assigned 'false' or 'true', which if bool is replaced by gnulib are still guaranteed to be 0 and 1, so using it directly as an array index is still safe. But the thing is that due to -Dbool=char the variable is declared char, not bool. But if the variable contains exactly 0 or 1, it still makes no difference as an array index whether it was typed bool or char. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On Wed, Dec 18, 2013 at 06:09:40AM -0700, Eric Blake wrote: On 12/18/2013 02:35 AM, Martin Kletzander wrote: When test-locking is enabled, CFLAGS have -Dbool=char, which makes Huh? bool should never be defined to char in CFLAGS; it should only be replaced if gnulib thinks the compiler is too old. pipefd[bool] fail (obviously). Forcing the subscript to be bool by double negation fixes the build breaker. I don't get how this could possibly make a difference. The 'output' variable is declared bool, and then only ever assigned 'false' or 'true', which if bool is replaced by gnulib are still guaranteed to be 0 and 1, so using it directly as an array index is still safe. Signed-off-by: Martin Kletzander mklet...@redhat.com --- Notes: I still can't build with '--enable-test-locking' and I'm not sure this is the proper way to fix it. Also, the code wasn't touched for months, so I'm not pushing it as a build-breaker. NACK. I need more details about the actual failure you are seeing, but this is not the right patch. The reason for this is that CIL didn't understand the bool type. GNULIB's automagic detection didn't help here because the build is still done by gcc which supports it just fine. We use -save-temps and then CIL analyses those intermediate files and threw up. I'll see if it is still a problem on F20 CIL. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] build: fix building with '--enable-test-locking'
On Wed, Dec 18, 2013 at 06:34:43AM -0700, Eric Blake wrote: On 12/18/2013 06:25 AM, Martin Kletzander wrote: pipefd[bool] fail (obviously). Forcing the subscript to be bool by double negation fixes the build breaker. I don't get how this could possibly make a difference. The 'output' variable is declared bool, and then only ever assigned 'false' or 'true', which if bool is replaced by gnulib are still guaranteed to be 0 and 1, so using it directly as an array index is still safe. But the thing is that due to -Dbool=char the variable is declared char, not bool. But if the variable contains exactly 0 or 1, it still makes no difference as an array index whether it was typed bool or char. Yep, but '--enable-werror' is sad aboot it. Anyways, the code for test-locking is way too old and I don't need it in for anything right now, so I'm ok with keeping everything as-is. Thanks for having a look at it and sorry for wasting your time. Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
