| Message ID | 1309152553-25693-1-git-send-email-weil@mail.berlios.de |
|---|---|
| State | Accepted |
| Headers | show |
Am 27.06.2011 07:29, schrieb Stefan Weil: > Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation > with --enable-vnc-png, but broke it with --enable-vnc-png. should be with --enable-vnc-jpeg, but broke it with --enable-vnc-png. Please fix this detail before applying the patch. Thanks, Stefan
On 2011-06-27 07:29, Stefan Weil wrote: > Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation > with --enable-vnc-png, but broke it with --enable-vnc-png. > > The breakage is caused by pngconfig.h which checks whether > setjmp.h was already included and fails because qemu-common.h > includes setjmp.h. > > The check is disabled by defining PNG_SKIP_SETJMP_CHECK. Did you check if "You can bypass this test if you know that your application uses exactly the same setjmp.h that was included when libpng was built." (from /usr/include/pngconf.h) applies for us in all supported cases? Jan > > Cc: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > ui/vnc-enc-tight.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c > index 6d36a7f..5c02803 100644 > --- a/ui/vnc-enc-tight.c > +++ b/ui/vnc-enc-tight.c > @@ -34,6 +34,9 @@ > #include "qemu-common.h" > > #ifdef CONFIG_VNC_PNG > +/* The following define is needed by pngconf.h. Otherwise it won't compile, > + because setjmp.h was already included by qemu-common.h. */ > +#define PNG_SKIP_SETJMP_CHECK > #include <png.h> > #endif > #ifdef CONFIG_VNC_JPEG
On Mon, Jun 27, 2011 at 6:29 AM, Stefan Weil <weil@mail.berlios.de> wrote: > Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation > with --enable-vnc-png, but broke it with --enable-vnc-png. > > The breakage is caused by pngconfig.h which checks whether > setjmp.h was already included and fails because qemu-common.h > includes setjmp.h. > > The check is disabled by defining PNG_SKIP_SETJMP_CHECK. > > Cc: Blue Swirl <blauwirbel@gmail.com> > Signed-off-by: Stefan Weil <weil@mail.berlios.de> > --- > ui/vnc-enc-tight.c | 3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Some more info on why Stefan Weil's fix is correct: The setjmp(3) man page says, "POSIX does not specify whether setjmp() will save the signal mask. In System V it will not. In 4.3BSD it will, and there is a function _setjmp that will not. By default, Linux/glibc follows the System V behavior, but the BSD behavior is provided if the _BSD_SOURCE feature test macro is defined and none of _POSIX_SOURCE, _POSIX_C_SOURCE, _XOPEN_SOURCE, _XOPEN_SOURCE_EXTENDED, _GNU_SOURCE, or _SVID_SOURCE is defined." Apparently libpng wants to make sure that the setjmp() which will not save signal masks is used on Linux. The problem is that Linux supports both versions and jmp_buf is part of the libpng API. That means the application and the library need to agree on which setjmp() semantics will be used. That said, QEMU doesn't seem to make use of the jmp_buf API in libpng, so this really shouldn't matter at all. We can skip the header check. Stefan
On 2011-06-27 08:10, Stefan Hajnoczi wrote: > On Mon, Jun 27, 2011 at 6:29 AM, Stefan Weil <weil@mail.berlios.de> wrote: >> Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation >> with --enable-vnc-png, but broke it with --enable-vnc-png. >> >> The breakage is caused by pngconfig.h which checks whether >> setjmp.h was already included and fails because qemu-common.h >> includes setjmp.h. >> >> The check is disabled by defining PNG_SKIP_SETJMP_CHECK. >> >> Cc: Blue Swirl <blauwirbel@gmail.com> >> Signed-off-by: Stefan Weil <weil@mail.berlios.de> >> --- >> ui/vnc-enc-tight.c | 3 +++ >> 1 files changed, 3 insertions(+), 0 deletions(-) > > Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> > > Some more info on why Stefan Weil's fix is correct: > > The setjmp(3) man page says, > > "POSIX does not specify whether setjmp() will save the signal > mask. In System V it will not. In 4.3BSD it will, and there is a > function _setjmp that will not. By default, Linux/glibc follows the > System V behavior, but the BSD behavior is provided if the > _BSD_SOURCE feature test macro is defined and none of > _POSIX_SOURCE, _POSIX_C_SOURCE, _XOPEN_SOURCE, > _XOPEN_SOURCE_EXTENDED, _GNU_SOURCE, or _SVID_SOURCE is defined." > > Apparently libpng wants to make sure that the setjmp() which will not > save signal masks is used on Linux. The problem is that Linux > supports both versions and jmp_buf is part of the libpng API. That > means the application and the library need to agree on which setjmp() > semantics will be used. > > That said, QEMU doesn't seem to make use of the jmp_buf API in libpng, > so this really shouldn't matter at all. We can skip the header check. That sounds good. Please adjust the comment that this patch introduces. Jan
Stefan Weil <weil@mail.berlios.de> writes: > Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation > with --enable-vnc-png, but broke it with --enable-vnc-png. You mean "with --enable-vnc-jpeg, but", don't you? > The breakage is caused by pngconfig.h which checks whether > setjmp.h was already included and fails because qemu-common.h > includes setjmp.h. > > The check is disabled by defining PNG_SKIP_SETJMP_CHECK. [...]
On 06/27/2011 12:44 AM, Stefan Weil wrote: > Am 27.06.2011 07:29, schrieb Stefan Weil: >> Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation >> with --enable-vnc-png, but broke it with --enable-vnc-png. > > should be > > with --enable-vnc-jpeg, but broke it with --enable-vnc-png. > > Please fix this detail before applying the patch. Applied. Thanks. Regards, Anthony Liguori > > Thanks, > Stefan > >
diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c index 6d36a7f..5c02803 100644 --- a/ui/vnc-enc-tight.c +++ b/ui/vnc-enc-tight.c @@ -34,6 +34,9 @@ #include "qemu-common.h" #ifdef CONFIG_VNC_PNG +/* The following define is needed by pngconf.h. Otherwise it won't compile, + because setjmp.h was already included by qemu-common.h. */ +#define PNG_SKIP_SETJMP_CHECK #include <png.h> #endif #ifdef CONFIG_VNC_JPEG
Commit f26e428da505709ec03b2ed2c9eb3db82b30bd7b fixed compilation with --enable-vnc-png, but broke it with --enable-vnc-png. The breakage is caused by pngconfig.h which checks whether setjmp.h was already included and fails because qemu-common.h includes setjmp.h. The check is disabled by defining PNG_SKIP_SETJMP_CHECK. Cc: Blue Swirl <blauwirbel@gmail.com> Signed-off-by: Stefan Weil <weil@mail.berlios.de> --- ui/vnc-enc-tight.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)