Re: -Werror vs. NetBSD

[Martin Thomson]

Yes,  the landing of the first patches for TLS 1.3 was a bit messy.  We are
working on fixing these problems.  We do have a requirement for c99, which
might account for the //comments.  We are trying not to land any of those
though.
On Feb 4, 2016 8:27 PM, "Thomas Klausner"  wrote:

> Hi Martin!
>
> Thanks for the reply.
>
> Yes, of course I can file a bug report for that. I guess I should
> finish the patch first?
>
> For two days now, I see a different build failure though:
>
> gcc -o NetBSD7.99.26_64_OPT.OBJ/tls13hkdf.o -c -O -fPIC -DPIC  -ansi -Wall
> -Wno-switch -pipe -DNETBSD -Dunix -DHAVE_STRERROR -DHAVE_BSD_FLOCK -Wall
> -Werror -DXP_UNIX -UDEBUG -DNDEBUG -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT
> -DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNSS_ENABLE_SSL_ZLIB
> -I/usr/pkg/include/nspr -I/usr/X11R6/include
> -I./../dist/NetBSD7.99.26_64_OPT.OBJ/include -I./../dist/public/
> -I./../dist/private/ -I/usr/X11R6/include
> -I../../dist/NetBSD7.99.26_64_OPT.OBJ/include -I../../dist/public/
> -I../../dist/private/ -I/usr/X11R6/include
> -I../../../dist/NetBSD7.99.26_64_OPT.OBJ/include -I../../../dist/public/nss
> -I../../../dist/private/nss  tls13hkdf.c
> tls13hkdf.c:15:1: error: expected identifier or '(' before '/' token
>  // TODO(e...@rtfm.com): Export this separately.
>  ^
> tls13hkdf.c:15:12: error: stray '@' in program
>  // TODO(e...@rtfm.com): Export this separately.
> ^
> tls13hkdf.c: In function 'tls13_HkdfExpandLabel':
> tls13hkdf.c:136:5: error: implicit declaration of function
> 'tls13_EncodeUintX' [-Werror=implicit-function-declaration]
>  ptr = tls13_EncodeUintX(keySize, 2, ptr);
>  ^
> tls13hkdf.c:136:9: error: assignment makes pointer from integer without a
> cast [-Werror]
>  ptr = tls13_EncodeUintX(keySize, 2, ptr);
>  ^
> tls13hkdf.c:137:9: error: assignment makes pointer from integer without a
> cast [-Werror]
>  ptr = tls13_EncodeUintX(labelLen + kLabelPrefixLen, 1, ptr);
>  ^
> tls13hkdf.c:142:9: error: assignment makes pointer from integer without a
> cast [-Werror]
>  ptr = tls13_EncodeUintX(handshakeHashLen, 1, ptr);
>  ^
> cc1: all warnings being treated as errors
>
>  Thomas
>
>
> On Mon, Feb 01, 2016 at 11:15:14AM +1100, Martin Thomson wrote:
> > Hi Thomas,
> >
> > Do you think that you could push these patches to bugzilla?  See
> >
> https://bugzilla.mozilla.org/enter_bug.cgi?product=NSS=Libraries
> >
> > And it would be easier to review this as a single patch, I think,
> > since all the changes are fairly simple.
> >
> > On Sat, Jan 30, 2016 at 11:40 PM, Thomas Klausner  wrote:
> > > Hi!
> > >
> > > Recently nss turned on -Werror by default for all platforms. I think
> > > that's a good idea. However, it majorly broke the build on NetBSD with
> > > lots of these types of messages:
> > >
> > > certcgi.c: In function 'MakeNameConstraints':
> > > certcgi.c:1654:6: error: array subscript has type 'char'
> [-Werror=char-subscripts]
> > >
> > > The reason is that NetBSD is very picky about the use of the ctype
> > > functions. To cite the man page ctype(3):
> > >
> > > CAVEATS
> > >  The first argument of these functions is of type int, but only a
> very
> > >  restricted subset of values are actually valid.  The argument
> must either
> > >  be the value of the macro EOF (which has a negative value), or
> must be a
> > >  non-negative value within the range representable as unsigned
> char.
> > >  Passing invalid values leads to undefined behavior.
> > >
> > >  Values of type int that were returned by getc(3), fgetc(3), and
> similar
> > >  functions or macros are already in the correct range, and may be
> safely
> > >  passed to these ctype functions without any casts.
> > >
> > >  Values of type char or signed char must first be cast to unsigned
> char,
> > >  to ensure that the values are within the correct range.  Casting a
> > >  negative-valued char or signed char directly to int will produce a
> > >  negative-valued int, which will be outside the range of allowed
> values
> > >  (unless it happens to be equal to EOF, but even that would not
> give the
> > >  desired result).
> > >
> > >
> > > I've started fixing these, attached is a first batch of patches. More
> > > are needed, but I wanted to find out first how to make sure they get
> > > applied.
> > >
> > > I'll also attach two patches for the included zlib. It doesn't compile
> > > because of missing prototypes for read/write/close, so I added an
> > > #include  in two places.
> > >
> > > Thanks,
> > >  Thomas
> > >
> > > --
> > > dev-tech-crypto mailing list
> > > dev-tech-crypto@lists.mozilla.org
> > > https://lists.mozilla.org/listinfo/dev-tech-crypto
> > --
> > dev-tech-crypto mailing list
> > dev-tech-crypto@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-tech-crypto
> >
>
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org

Re: -Werror vs. NetBSD

[Thomas Klausner]

Hi Martin!

Thanks for the reply.

Yes, of course I can file a bug report for that. I guess I should
finish the patch first?

For two days now, I see a different build failure though:

gcc -o NetBSD7.99.26_64_OPT.OBJ/tls13hkdf.o -c -O -fPIC -DPIC  -ansi -Wall 
-Wno-switch -pipe -DNETBSD -Dunix -DHAVE_STRERROR -DHAVE_BSD_FLOCK -Wall 
-Werror -DXP_UNIX -UDEBUG -DNDEBUG -DUSE_UTIL_DIRECTLY -DNO_NSPR_10_SUPPORT 
-DSSL_DISABLE_DEPRECATED_CIPHER_SUITE_NAMES -DNSS_ENABLE_SSL_ZLIB 
-I/usr/pkg/include/nspr -I/usr/X11R6/include 
-I./../dist/NetBSD7.99.26_64_OPT.OBJ/include -I./../dist/public/ 
-I./../dist/private/ -I/usr/X11R6/include 
-I../../dist/NetBSD7.99.26_64_OPT.OBJ/include -I../../dist/public/ 
-I../../dist/private/ -I/usr/X11R6/include 
-I../../../dist/NetBSD7.99.26_64_OPT.OBJ/include -I../../../dist/public/nss 
-I../../../dist/private/nss  tls13hkdf.c
tls13hkdf.c:15:1: error: expected identifier or '(' before '/' token
 // TODO(e...@rtfm.com): Export this separately.
 ^
tls13hkdf.c:15:12: error: stray '@' in program
 // TODO(e...@rtfm.com): Export this separately.
^
tls13hkdf.c: In function 'tls13_HkdfExpandLabel':
tls13hkdf.c:136:5: error: implicit declaration of function 'tls13_EncodeUintX' 
[-Werror=implicit-function-declaration]
 ptr = tls13_EncodeUintX(keySize, 2, ptr);
 ^
tls13hkdf.c:136:9: error: assignment makes pointer from integer without a cast 
[-Werror]
 ptr = tls13_EncodeUintX(keySize, 2, ptr);
 ^
tls13hkdf.c:137:9: error: assignment makes pointer from integer without a cast 
[-Werror]
 ptr = tls13_EncodeUintX(labelLen + kLabelPrefixLen, 1, ptr);
 ^
tls13hkdf.c:142:9: error: assignment makes pointer from integer without a cast 
[-Werror]
 ptr = tls13_EncodeUintX(handshakeHashLen, 1, ptr);
 ^
cc1: all warnings being treated as errors

 Thomas


On Mon, Feb 01, 2016 at 11:15:14AM +1100, Martin Thomson wrote:
> Hi Thomas,
> 
> Do you think that you could push these patches to bugzilla?  See
> https://bugzilla.mozilla.org/enter_bug.cgi?product=NSS=Libraries
> 
> And it would be easier to review this as a single patch, I think,
> since all the changes are fairly simple.
> 
> On Sat, Jan 30, 2016 at 11:40 PM, Thomas Klausner  wrote:
> > Hi!
> >
> > Recently nss turned on -Werror by default for all platforms. I think
> > that's a good idea. However, it majorly broke the build on NetBSD with
> > lots of these types of messages:
> >
> > certcgi.c: In function 'MakeNameConstraints':
> > certcgi.c:1654:6: error: array subscript has type 'char' 
> > [-Werror=char-subscripts]
> >
> > The reason is that NetBSD is very picky about the use of the ctype
> > functions. To cite the man page ctype(3):
> >
> > CAVEATS
> >  The first argument of these functions is of type int, but only a very
> >  restricted subset of values are actually valid.  The argument must 
> > either
> >  be the value of the macro EOF (which has a negative value), or must be 
> > a
> >  non-negative value within the range representable as unsigned char.
> >  Passing invalid values leads to undefined behavior.
> >
> >  Values of type int that were returned by getc(3), fgetc(3), and similar
> >  functions or macros are already in the correct range, and may be safely
> >  passed to these ctype functions without any casts.
> >
> >  Values of type char or signed char must first be cast to unsigned char,
> >  to ensure that the values are within the correct range.  Casting a
> >  negative-valued char or signed char directly to int will produce a
> >  negative-valued int, which will be outside the range of allowed values
> >  (unless it happens to be equal to EOF, but even that would not give the
> >  desired result).
> >
> >
> > I've started fixing these, attached is a first batch of patches. More
> > are needed, but I wanted to find out first how to make sure they get
> > applied.
> >
> > I'll also attach two patches for the included zlib. It doesn't compile
> > because of missing prototypes for read/write/close, so I added an
> > #include  in two places.
> >
> > Thanks,
> >  Thomas
> >
> > --
> > dev-tech-crypto mailing list
> > dev-tech-crypto@lists.mozilla.org
> > https://lists.mozilla.org/listinfo/dev-tech-crypto
> -- 
> dev-tech-crypto mailing list
> dev-tech-crypto@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-crypto
> 
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto

Re: -Werror vs. NetBSD

[Martin Thomson]

Hi Thomas,

Do you think that you could push these patches to bugzilla?  See
https://bugzilla.mozilla.org/enter_bug.cgi?product=NSS=Libraries

And it would be easier to review this as a single patch, I think,
since all the changes are fairly simple.

On Sat, Jan 30, 2016 at 11:40 PM, Thomas Klausner  wrote:
> Hi!
>
> Recently nss turned on -Werror by default for all platforms. I think
> that's a good idea. However, it majorly broke the build on NetBSD with
> lots of these types of messages:
>
> certcgi.c: In function 'MakeNameConstraints':
> certcgi.c:1654:6: error: array subscript has type 'char' 
> [-Werror=char-subscripts]
>
> The reason is that NetBSD is very picky about the use of the ctype
> functions. To cite the man page ctype(3):
>
> CAVEATS
>  The first argument of these functions is of type int, but only a very
>  restricted subset of values are actually valid.  The argument must either
>  be the value of the macro EOF (which has a negative value), or must be a
>  non-negative value within the range representable as unsigned char.
>  Passing invalid values leads to undefined behavior.
>
>  Values of type int that were returned by getc(3), fgetc(3), and similar
>  functions or macros are already in the correct range, and may be safely
>  passed to these ctype functions without any casts.
>
>  Values of type char or signed char must first be cast to unsigned char,
>  to ensure that the values are within the correct range.  Casting a
>  negative-valued char or signed char directly to int will produce a
>  negative-valued int, which will be outside the range of allowed values
>  (unless it happens to be equal to EOF, but even that would not give the
>  desired result).
>
>
> I've started fixing these, attached is a first batch of patches. More
> are needed, but I wanted to find out first how to make sure they get
> applied.
>
> I'll also attach two patches for the included zlib. It doesn't compile
> because of missing prototypes for read/write/close, so I added an
> #include  in two places.
>
> Thanks,
>  Thomas
>
> --
> dev-tech-crypto mailing list
> dev-tech-crypto@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-tech-crypto
-- 
dev-tech-crypto mailing list
dev-tech-crypto@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-tech-crypto

-Werror vs. NetBSD

[Thomas Klausner]

Hi!

Recently nss turned on -Werror by default for all platforms. I think
that's a good idea. However, it majorly broke the build on NetBSD with
lots of these types of messages:

certcgi.c: In function 'MakeNameConstraints':
certcgi.c:1654:6: error: array subscript has type 'char' 
[-Werror=char-subscripts]

The reason is that NetBSD is very picky about the use of the ctype
functions. To cite the man page ctype(3):

CAVEATS
 The first argument of these functions is of type int, but only a very
 restricted subset of values are actually valid.  The argument must either
 be the value of the macro EOF (which has a negative value), or must be a
 non-negative value within the range representable as unsigned char.
 Passing invalid values leads to undefined behavior.

 Values of type int that were returned by getc(3), fgetc(3), and similar
 functions or macros are already in the correct range, and may be safely
 passed to these ctype functions without any casts.

 Values of type char or signed char must first be cast to unsigned char,
 to ensure that the values are within the correct range.  Casting a
 negative-valued char or signed char directly to int will produce a
 negative-valued int, which will be outside the range of allowed values
 (unless it happens to be equal to EOF, but even that would not give the
 desired result).


I've started fixing these, attached is a first batch of patches. More
are needed, but I wanted to find out first how to make sure they get
applied.

I'll also attach two patches for the included zlib. It doesn't compile
because of missing prototypes for read/write/close, so I added an
#include  in two places.

Thanks,
 Thomas
$NetBSD$

Missing ctype(3) casts.

--- cmd/fipstest/fipstest.c.orig2016-01-30 10:42:11.0 +
+++ cmd/fipstest/fipstest.c
@@ -127,7 +127,7 @@ from_hex_str(unsigned char *buf, unsigne
 
 /* count the hex digits */
 nxdigit = 0;
-for (nxdigit = 0; isxdigit(str[nxdigit]); nxdigit++) {
+for (nxdigit = 0; isxdigit((unsigned char)str[nxdigit]); nxdigit++) {
 /* empty body */
 }
 if (nxdigit == 0) {
@@ -335,7 +335,7 @@ tdea_kat_mmt(char *reqfn)
 /* NumKeys */
 if (strncmp([0], "NumKeys", 7) == 0) {
 i = 7;
-while (isspace(buf[i]) || buf[i] == '=') {
+while (isspace((unsigned char)buf[i]) || buf[i] == '=') {
 i++;
 }
 numKeys = buf[i];
@@ -359,10 +359,10 @@ tdea_kat_mmt(char *reqfn)
 if (numKeys == 0) {
 if (strncmp(buf, "KEYs", 4) == 0) {
 i = 4;
-while (isspace(buf[i]) || buf[i] == '=') {
+while (isspace((unsigned char)buf[i]) || buf[i] == '=') {
 i++;
 }
-for (j=0; isxdigit(buf[i]); i+=2,j++) {
+for (j=0; isxdigit((unsigned char)buf[i]); i+=2,j++) {
 hex_to_byteval([i], [j]);
 key[j+8] = key[j];
 key[j+16] = key[j];
@@ -374,10 +374,10 @@ tdea_kat_mmt(char *reqfn)
 /* KEY1 = ... */
 if (strncmp(buf, "KEY1", 4) == 0) {
 i = 4;
-while (isspace(buf[i]) || buf[i] == '=') {
+while (isspace((unsigned char)buf[i]) || buf[i] == '=') {
 i++;
 }
-for (j=0; isxdigit(buf[i]); i+=2,j++) {
+for (j=0; isxdigit((unsigned char)buf[i]); i+=2,j++) {
 hex_to_byteval([i], [j]);
 }
 fputs(buf, resp);
@@ -386,10 +386,10 @@ tdea_kat_mmt(char *reqfn)
 /* KEY2 = ... */
 if (strncmp(buf, "KEY2", 4) == 0) {
 i = 4;
-while (isspace(buf[i]) || buf[i] == '=') {
+while (isspace((unsigned char)buf[i]) || buf[i] == '=') {
 i++;
 }
-for (j=8; isxdigit(buf[i]); i+=2,j++) {
+for (j=8; isxdigit((unsigned char)buf[i]); i+=2,j++) {
 hex_to_byteval([i], [j]);
 }
 fputs(buf, resp);
@@ -398,10 +398,10 @@ tdea_kat_mmt(char *reqfn)
 /* KEY3 = ... */
 if (strncmp(buf, "KEY3", 4) == 0) {
 i = 4;
-while (isspace(buf[i]) || buf[i] == '=') {
+while (isspace((unsigned char)buf[i]) || buf[i] == '=') {
 i++;
 }
-for (j=16; isxdigit(buf[i]); i+=2,j++) {
+for (j=16; isxdigit((unsigned char)buf[i]); i+=2,j++) {
 hex_to_byteval([i], [j]);
 }
 fputs(buf, resp);
@@ -413,7 +413,7 @@ tdea_kat_mmt(char *reqfn)
 if (strncmp(buf, "IV", 2) == 0) {
 mode = NSS_DES_EDE3_CBC;
 i = 2;
-while (isspace(buf[i]) || buf[i] == '=') {
+while