[vlc-devel] Report on contrib tree clang issues
Rafaël Carré
funman at videolan.org
Tue Jan 17 06:10:57 CET 2012
Le 2012-01-16 21:57, Olivier Gambier a écrit :
> Le 17 janv. 2012 à 03:23, Rafaël Carré a écrit :
>> It would be better if you provide patches against contrib/ that we can
>> apply directly
>
>
> Ok.
better, but please send git formatted patches (see
http://wiki.videolan.org/Git )
> ********************************
> Libcaca
> ********************************
>
> diff --git a/contrib/src/caca/rules.mak b/contrib/src/caca/rules.mak
> index ebf37cb..04f7d6d 100644
> --- a/contrib/src/caca/rules.mak
> +++ b/contrib/src/caca/rules.mak
> @@ -16,6 +16,7 @@ caca: libcaca-$(CACA_VERSION).tar.gz .sum-caca
> $(UNPACK)
> ifdef HAVE_MACOSX
> $(APPLY) $(SRC)/caca/caca-osx-sdkofourchoice.patch
> + $(APPLY) $(SRC)/caca/caca-osx-alias.patch
> endif
> ifdef HAVE_WIN32
> $(APPLY) $(SRC)/caca/caca-win32-static.patch
>
>
>
>
> and the patch contrib/src/caca/caca-osx-alias.patch
>
> --- libcaca/caca/caca.h 2011-07-05 00:09:51.000000000 -0700
> +++ libcaca.new/caca/caca.h 2011-07-05 00:10:10.000000000 -0700
> @@ -645,7 +645,7 @@
> # define CACA_DEPRECATED
> # endif
>
> -# if defined __GNUC__ && __GNUC__ > 3
> +# if !defined __APPLE__ && defined __GNUC__ && __GNUC__ > 3
> # define CACA_ALIAS(x) __attribute__ ((weak, alias(#x)))
> # else
> # define CACA_ALIAS(x)
it should use __clang__ and it'd be nice if you send this patch upstream
>
> ********************************
> libgcrypt: same as suggested earlier by Felix Paul Kühne
> ********************************
>
>
> I guess it would be better to do these only with clang (via a ifdef HAVE_CLANG maybe?)
it's better to have a solution which works everywhere so it can be sent
upstream
> --- a/contrib/src/gcrypt/rules.mak
> +++ b/contrib/src/gcrypt/rules.mak
> @@ -11,19 +11,25 @@ $(TARBALLS)/libgcrypt-$(GCRYPT_VERSION).tar.bz2:
>
> libgcrypt: libgcrypt-$(GCRYPT_VERSION).tar.bz2 .sum-gcrypt
> $(UNPACK)
> +ifdef HAVE_MACOSX
> + $(APPLY) $(SRC)/gcrypt/gcrypt-osx-clang-asm.patch
> +endif
> $(MOVE)
>
> DEPS_gcrypt = gpg-error
>
> CONFIGURE_OPTS =
> +MAKE_OPTS =
> ifdef HAVE_WIN64
> CONFIGURE_OPTS += --disable-asm
> endif
> ifdef HAVE_MACOSX
> -CONFIGURE_OPTS += --disable-aesni-support
> +CONFIGURE_OPTS += --disable-aesni-support --disable-asm
> +MAKE_OPTS = "CFLAGS=-fheinous-gnu-extensions -std=gnu89"
what do these do?
-std=gnu89 should work with gcc too (isn't it the default?)
> endif
> .gcrypt: libgcrypt
> #$(RECONF)
> cd $< && $(HOSTVARS) ./configure $(HOSTCONF) --enable-ciphers=aes,des,rfc2268,arcfour --enable-digests=sha
> + cd $< && $(MAKE) $(MAKE_OPTS)
> cd $< && $(MAKE) install
> touch $@
>
>
>
> and the patch from homebrew:
>
>
> cat contrib/src/gcrypt/gcrypt-osx-clang-asm.patch
>
> --- libgrcypt/cipher/rijndael.c 2011-08-19 09:16:14.000000000 -0700
> +++ libgcrypt.new/cipher/rijndael.c 2011-08-19 09:19:01.000000000 -0700
> @@ -730,13 +730,13 @@ do_aesni_enc_aligned (const RIJNDAEL_con
> "movdqa 0x90(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xa0(%%esi), %%xmm1\n\t"
> - "cmp $10, %[rounds]\n\t"
> + "cmpl $10, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xb0(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xc0(%%esi), %%xmm1\n\t"
> - "cmp $12, %[rounds]\n\t"
> + "cmpl $12, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xd0(%%esi), %%xmm1\n\t"
> @@ -785,13 +785,13 @@ do_aesni_dec_aligned (const RIJNDAEL_con
> "movdqa 0x90(%%esi), %%xmm1\n\t"
> aesdec_xmm1_xmm0
> "movdqa 0xa0(%%esi), %%xmm1\n\t"
> - "cmp $10, %[rounds]\n\t"
> + "cmpl $10, %[rounds]\n\t"
> "jz .Ldeclast%=\n\t"
> aesdec_xmm1_xmm0
> "movdqa 0xb0(%%esi), %%xmm1\n\t"
> aesdec_xmm1_xmm0
> "movdqa 0xc0(%%esi), %%xmm1\n\t"
> - "cmp $12, %[rounds]\n\t"
> + "cmpl $12, %[rounds]\n\t"
> "jz .Ldeclast%=\n\t"
> aesdec_xmm1_xmm0
> "movdqa 0xd0(%%esi), %%xmm1\n\t"
> @@ -844,13 +844,13 @@ do_aesni_cfb (const RIJNDAEL_context *ct
> "movdqa 0x90(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xa0(%%esi), %%xmm1\n\t"
> - "cmp $10, %[rounds]\n\t"
> + "cmpl $10, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xb0(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xc0(%%esi), %%xmm1\n\t"
> - "cmp $12, %[rounds]\n\t"
> + "cmpl $12, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xd0(%%esi), %%xmm1\n\t"
> @@ -862,7 +862,7 @@ do_aesni_cfb (const RIJNDAEL_context *ct
> "movdqu %[src], %%xmm1\n\t" /* Save input. */
> "pxor %%xmm1, %%xmm0\n\t" /* xmm0 = input ^ IV */
>
> - "cmp $1, %[decrypt]\n\t"
> + "cmpl $1, %[decrypt]\n\t"
> "jz .Ldecrypt_%=\n\t"
> "movdqa %%xmm0, %[iv]\n\t" /* [encrypt] Store IV. */
> "jmp .Lleave_%=\n"
> @@ -923,13 +923,13 @@ do_aesni_ctr (const RIJNDAEL_context *ct
> "movdqa 0x90(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xa0(%%esi), %%xmm1\n\t"
> - "cmp $10, %[rounds]\n\t"
> + "cmpl $10, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xb0(%%esi), %%xmm1\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xc0(%%esi), %%xmm1\n\t"
> - "cmp $12, %[rounds]\n\t"
> + "cmpl $12, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> "movdqa 0xd0(%%esi), %%xmm1\n\t"
> @@ -1050,7 +1050,7 @@ do_aesni_ctr_4 (const RIJNDAEL_context *
> aesenc_xmm1_xmm3
> aesenc_xmm1_xmm4
> "movdqa 0xa0(%%esi), %%xmm1\n\t"
> - "cmp $10, %[rounds]\n\t"
> + "cmpl $10, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> aesenc_xmm1_xmm2
> @@ -1062,7 +1062,7 @@ do_aesni_ctr_4 (const RIJNDAEL_context *
> aesenc_xmm1_xmm3
> aesenc_xmm1_xmm4
> "movdqa 0xc0(%%esi), %%xmm1\n\t"
> - "cmp $12, %[rounds]\n\t"
> + "cmpl $12, %[rounds]\n\t"
> "jz .Lenclast%=\n\t"
> aesenc_xmm1_xmm0
> aesenc_xmm1_xmm2
I dont know much x86 asm, does cmpl work fine with gcc too ?
>
>
>
>
>
>
> ********************************
> zvbi:
> ********************************
>
>
> diff --git a/contrib/src/zvbi/rules.mak b/contrib/src/zvbi/rules.mak
> index 50f372e..7795629 100644
> --- a/contrib/src/zvbi/rules.mak
> +++ b/contrib/src/zvbi/rules.mak
> @@ -21,6 +21,8 @@ zvbi: zvbi-$(ZVBI_VERSION).tar.bz2 .sum-zvbi
> ifdef HAVE_WIN32
> $(APPLY) $(SRC)/zvbi/zvbi-win32.patch
> endif
> +# From: http://www.freebsd.org/cgi/getmsg.cgi?fetch=813399+0+/usr/local/www/db/text/2011/cvs-all/20111218.cvs-all
> + $(APPLY) $(SRC)/zvbi/zvbi-clang.patch
> $(MOVE)
>
> DEPS_zvbi = pthreads iconv $(DEPS_iconv)
>
>
>
>
> And the patch from bsd ports (for the nested function):
>
>
>
> cat contrib/src/zvbi/zvbi-clang.patch
>
>
> --- zvbi/src/packet.c 2011-12-14 22:00:49.000000000 +0100
> +++ zvbi.new/src/packet.c 2011-12-14 22:00:53.000000000 +0100
> @@ -1747,24 +1747,22 @@
> int i, j, err = 0;
>
> /* XXX nested function not portable, to be removed */
> - int
> - bits(int count)
> - {
> - int r, n;
> -
> - r = buf;
> -
> - if ((n = count - left) > 0) {
> - r |= (buf = *triplet++) << left;
> - left = 18;
> - } else
> - n = count;
> -
> - buf >>= n;
> - left -= n;
> -
> - return r & ((1UL << count) - 1);
> - }
> +#define bits(count) ({ \
> + int r, n; \
> + \
> + r = buf; \
> + \
> + if ((n = count - left) > 0) { \
> + r |= (buf = *triplet++) << left; \
> + left = 18; \
> + } else \
> + n = count; \
> + \
> + buf >>= n; \
> + left -= n; \
> + \
> + (r & ((1UL << count) - 1)); \
> + })
>
> if ((designation = vbi_unham8 (*p)) < 0)
> return FALSE;
>
>
> --- zvbi/src/teletext.c 2011-12-14 22:01:23.000000000 +0100
> +++ zvbi.new/src/teletext.c 2011-12-14 22:01:30.000000000 +0100
> @@ -1258,180 +1258,177 @@
> int pdc_hr;
>
> /* XXX nested function not portable, to be removed */
> - void
> - flush(int column)
> - {
> - int row = inv_row + active_row;
> - int i;
> -
> - if (row >= ROWS)
> - return;
> -
> - if (type == OBJECT_TYPE_PASSIVE && !mac.unicode) {
> - active_column = column;
> - return;
> - }
> -
> - printv("flush [%04x%c,F%d%c,B%d%c,S%d%c,O%d%c,H%d%c] %d ... %d\n",
> - ac.unicode, mac.unicode ? '*' : ' ',
> - ac.foreground, mac.foreground ? '*' : ' ',
> - ac.background, mac.background ? '*' : ' ',
> - ac.size, mac.size ? '*' : ' ',
> - ac.opacity, mac.opacity ? '*' : ' ',
> - ac.flash, mac.flash ? '*' : ' ',
> - active_column, column - 1);
> -
> - for (i = inv_column + active_column; i < inv_column + column;) {
> - vbi_char c;
> -
> - if (i > 39)
> - break;
> -
> - c = acp[i];
> -
> - if (mac.underline) {
> - int u = ac.underline;
> -
> - if (!mac.unicode)
> - ac.unicode = c.unicode;
> -
> - if (vbi_is_gfx(ac.unicode)) {
> - if (u)
> - ac.unicode &= ~0x20; /* separated */
> - else
> - ac.unicode |= 0x20; /* contiguous */
> - mac.unicode = ~0;
> - u = 0;
> - }
> -
> - c.underline = u;
> - }
> - if (mac.foreground)
> - c.foreground = (ac.foreground != VBI_TRANSPARENT_BLACK) ?
> - ac.foreground : (row_color_transparent) ?
> - VBI_TRANSPARENT_BLACK : row_color;
> - if (mac.background)
> - c.background = (ac.background != VBI_TRANSPARENT_BLACK) ?
> - ac.background : (row_color_transparent) ?
> - VBI_TRANSPARENT_BLACK : row_color;
> - if (invert) {
> - int t = c.foreground;
> -
> - c.foreground = c.background;
> - c.background = t;
> - }
> - if (mac.opacity)
> - c.opacity = ac.opacity;
> - if (mac.flash)
> - c.flash = ac.flash;
> - if (mac.conceal)
> - c.conceal = ac.conceal;
> - if (mac.unicode) {
> - c.unicode = ac.unicode;
> - mac.unicode = 0;
> -
> - if (mac.size)
> - c.size = ac.size;
> - else if (c.size > VBI_DOUBLE_SIZE)
> - c.size = VBI_NORMAL_SIZE;
> - }
> -
> - acp[i] = c;
> -
> - if (type == OBJECT_TYPE_PASSIVE)
> - break;
> -
> - i++;
> -
> - if (type != OBJECT_TYPE_PASSIVE
> - && type != OBJECT_TYPE_ADAPTIVE) {
> - int raw;
> -
> - raw = (row == 0 && i < 9) ?
> - 0x20 : vbi_unpar8 (vtp->data.lop.raw[row][i - 1]);
> -
> - /* set-after spacing attributes cancelling non-spacing */
> -
> - switch (raw) {
> - case 0x00 ... 0x07: /* alpha + foreground color */
> - case 0x10 ... 0x17: /* mosaic + foreground color */
> - printv("... fg term %d %02x\n", i, raw);
> - mac.foreground = 0;
> - mac.conceal = 0;
> - break;
> -
> - case 0x08: /* flash */
> - mac.flash = 0;
> - break;
> -
> - case 0x0A: /* end box */
> - case 0x0B: /* start box */
> - if (i < COLUMNS && vbi_unpar8 (vtp->data.lop.raw[row][i]) == raw) {
> - printv("... boxed term %d %02x\n", i, raw);
> - mac.opacity = 0;
> - }
> -
> - break;
> -
> - case 0x0D: /* double height */
> - case 0x0E: /* double width */
> - case 0x0F: /* double size */
> - printv("... size term %d %02x\n", i, raw);
> - mac.size = 0;
> - break;
> - }
> -
> - if (i > 39)
> - break;
> -
> - raw = (row == 0 && i < 8) ?
> - 0x20 : vbi_unpar8 (vtp->data.lop.raw[row][i]);
> -
> - /* set-at spacing attributes cancelling non-spacing */
> -
> - switch (raw) {
> - case 0x09: /* steady */
> - mac.flash = 0;
> - break;
> -
> - case 0x0C: /* normal size */
> - printv("... size term %d %02x\n", i, raw);
> - mac.size = 0;
> - break;
> -
> - case 0x18: /* conceal */
> - mac.conceal = 0;
> - break;
> -
> - /*
> - * Non-spacing underlined/separated display attribute
> - * cannot be cancelled by a subsequent spacing attribute.
> - */
> -
> - case 0x1C: /* black background */
> - case 0x1D: /* new background */
> - printv("... bg term %d %02x\n", i, raw);
> - mac.background = 0;
> - break;
> - }
> - }
> - }
> -
> - active_column = column;
> - }
> +#define flush(column) \
> + ({ \
> + int row = inv_row + active_row; \
> + int i; \
> + \
> + if (row >= ROWS) \
> + break; \
> + \
> + if (type == OBJECT_TYPE_PASSIVE && !mac.unicode) { \
> + active_column = column; \
> + break; \
> + } \
> + \
> + printv("flush [%04x%c,F%d%c,B%d%c,S%d%c,O%d%c,H%d%c] %d ... %d\n", \
> + ac.unicode, mac.unicode ? '*' : ' ', \
> + ac.foreground, mac.foreground ? '*' : ' ', \
> + ac.background, mac.background ? '*' : ' ', \
> + ac.size, mac.size ? '*' : ' ', \
> + ac.opacity, mac.opacity ? '*' : ' ', \
> + ac.flash, mac.flash ? '*' : ' ', \
> + active_column, column - 1); \
> + \
> + for (i = inv_column + active_column; i < inv_column + column;) { \
> + vbi_char c; \
> + \
> + if (i > 39) \
> + break; \
> + \
> + c = acp[i]; \
> + \
> + if (mac.underline) { \
> + int u = ac.underline; \
> + \
> + if (!mac.unicode) \
> + ac.unicode = c.unicode; \
> + \
> + if (vbi_is_gfx(ac.unicode)) { \
> + if (u) \
> + ac.unicode &= ~0x20; /* separated */ \
> + else \
> + ac.unicode |= 0x20; /* contiguous */ \
> + mac.unicode = ~0; \
> + u = 0; \
> + } \
> + \
> + c.underline = u; \
> + } \
> + if (mac.foreground) \
> + c.foreground = (ac.foreground != VBI_TRANSPARENT_BLACK) ? \
> + ac.foreground : (row_color_transparent) ? \
> + VBI_TRANSPARENT_BLACK : row_color; \
> + if (mac.background) \
> + c.background = (ac.background != VBI_TRANSPARENT_BLACK) ? \
> + ac.background : (row_color_transparent) ? \
> + VBI_TRANSPARENT_BLACK : row_color; \
> + if (invert) { \
> + int t = c.foreground; \
> + \
> + c.foreground = c.background; \
> + c.background = t; \
> + } \
> + if (mac.opacity) \
> + c.opacity = ac.opacity; \
> + if (mac.flash) \
> + c.flash = ac.flash; \
> + if (mac.conceal) \
> + c.conceal = ac.conceal; \
> + if (mac.unicode) { \
> + c.unicode = ac.unicode; \
> + mac.unicode = 0; \
> + \
> + if (mac.size) \
> + c.size = ac.size; \
> + else if (c.size > VBI_DOUBLE_SIZE) \
> + c.size = VBI_NORMAL_SIZE; \
> + } \
> + \
> + acp[i] = c; \
> + \
> + if (type == OBJECT_TYPE_PASSIVE) \
> + break; \
> + \
> + i++; \
> + \
> + if (type != OBJECT_TYPE_PASSIVE \
> + && type != OBJECT_TYPE_ADAPTIVE) { \
> + int raw; \
> + \
> + raw = (row == 0 && i < 9) ? \
> + 0x20 : vbi_unpar8 (vtp->data.lop.raw[row][i - 1]); \
> + \
> + /* set-after spacing attributes cancelling non-spacing */ \
> + \
> + switch (raw) { \
> + case 0x00 ... 0x07: /* alpha + foreground color */ \
> + case 0x10 ... 0x17: /* mosaic + foreground color */ \
> + printv("... fg term %d %02x\n", i, raw); \
> + mac.foreground = 0; \
> + mac.conceal = 0; \
> + break; \
> + \
> + case 0x08: /* flash */ \
> + mac.flash = 0; \
> + break; \
> + \
> + case 0x0A: /* end box */ \
> + case 0x0B: /* start box */ \
> + if (i < COLUMNS && vbi_unpar8 (vtp->data.lop.raw[row][i]) == raw) { \
> + printv("... boxed term %d %02x\n", i, raw); \
> + mac.opacity = 0; \
> + } \
> + \
> + break; \
> + \
> + case 0x0D: /* double height */ \
> + case 0x0E: /* double width */ \
> + case 0x0F: /* double size */ \
> + printv("... size term %d %02x\n", i, raw); \
> + mac.size = 0; \
> + break; \
> + } \
> + \
> + if (i > 39) \
> + break; \
> + \
> + raw = (row == 0 && i < 8) ? \
> + 0x20 : vbi_unpar8 (vtp->data.lop.raw[row][i]); \
> + \
> + /* set-at spacing attributes cancelling non-spacing */ \
> + \
> + switch (raw) { \
> + case 0x09: /* steady */ \
> + mac.flash = 0; \
> + break; \
> + \
> + case 0x0C: /* normal size */ \
> + printv("... size term %d %02x\n", i, raw); \
> + mac.size = 0; \
> + break; \
> + \
> + case 0x18: /* conceal */ \
> + mac.conceal = 0; \
> + break; \
> + \
> + /* \
> + * Non-spacing underlined/separated display attribute \
> + * cannot be cancelled by a subsequent spacing attribute. \
> + */ \
> + \
> + case 0x1C: /* black background */ \
> + case 0x1D: /* new background */ \
> + printv("... bg term %d %02x\n", i, raw); \
> + mac.background = 0; \
> + break; \
> + } \
> + } \
> + } \
> + \
> + active_column = column; \
> + })
>
> /* XXX nested function not portable, to be removed */
> - void
> - flush_row(void)
> - {
> - if (type == OBJECT_TYPE_PASSIVE || type == OBJECT_TYPE_ADAPTIVE)
> - flush(active_column + 1);
> - else
> - flush(COLUMNS);
> -
> - if (type != OBJECT_TYPE_PASSIVE)
> - memset(&mac, 0, sizeof(mac));
> - }
> +#define flush_row do {\
> + if (type == OBJECT_TYPE_PASSIVE || type == OBJECT_TYPE_ADAPTIVE) \
> + flush(active_column + 1); \
> + else \
> + flush(COLUMNS); \
> +\
> + if (type != OBJECT_TYPE_PASSIVE) \
> + memset(&mac, 0, sizeof(mac)); \
> + } while (0)
>
> active_column = 0;
> active_row = 0;
> @@ -1554,7 +1551,7 @@
> printv("enh set_active row %d col %d\n", row, column);
>
> if (row > active_row)
> - flush_row();
> + flush_row;
>
> active_row = row;
> active_column = column;
> @@ -1750,7 +1747,7 @@
> break;
>
> case 0x15 ... 0x17: /* object definition */
> - flush_row();
> + flush_row;
> printv("enh obj definition 0x%02x 0x%02x\n", p->mode, p->data);
> printv("enh terminated\n");
> goto swedish;
> @@ -1766,7 +1763,7 @@
> case 0x1F: /* termination marker */
> default:
> terminate:
> - flush_row();
> + flush_row;
> printv("enh terminated %02x\n", p->mode);
> goto swedish;
> }
i guess this one is ok but i didn't check it. jpsaman?
btw zvbi is now unmaintained i think
More information about the vlc-devel
mailing list