[389-devel] Re: Sign compare checking
Date: Mon, 29 Aug 2016 11:43:27 -0600 From: Rich Megginson <rmegg...@redhat.com> Subject: [389-devel] Re: Sign compare checking To: 389-devel@lists.fedoraproject.org Message-ID: <a7fdb987-8efa-d26d-08c1-3cf0ecb59...@redhat.com> Content-Type: multipart/alternative; boundary=" 2A83E5142B304396ABB138B7" Part of the problem is that we wanted to support being able to use both mozldap and openldap, without too much "helper" code/macros/#ifdef MOZLDAP/etc. It looks as though this is a place where we need to have some sort of helper. (as for why we still support mozldap - we still need an ldap c sdk that supports NSS for crypto until we can fix that in the server. Once we change 389 so that it can use openldap with openssl/gnutls for crypto, we should consider deprecating support for mozldap.) What support for MozNSS is OpenLDAP lacking? Support for MozNSS has been in there for ages now. Patches to make Mozilla work with OpenLDAP instead of mozldap have also been available for ages. https://bugzilla.mozilla.org/show_bug.cgi?id=292127#c17 through comment #30 -- -- Howard Chu CTO, Symas Corp. http://www.symas.com Director, Highland Sun http://highlandsun.com/hyc/ Chief Architect, OpenLDAP http://www.openldap.org/project/ -- 389-devel mailing list 389-devel@lists.fedoraproject.org https://lists.fedoraproject.org/admin/lists/389-devel@lists.fedoraproject.org
[389-devel] Re: Sign compare checking
> > > > int ber_printf(BerElement *ber, const char *fmt, ...); > > > > lber.h:79:#define LBER_ERROR((ber_tag_t) -1) > > > > We check if (ber_printf(...) != LBER_ERROR) > > > > Of course, we can't satisfy either. We can't cast the LBER_ERROR from > > uint -> int without changing the value of it, and we can't cast the > > output of ber_printf from int -> uint, again, without potentially > > changing the value of it. So it seems that the openldap library may be > > impossible to satisfy the gcc type checking with -Wsign-compare. > > > > For now, I may just avoid these in my fixes, as it seems like a whole > > set of landmines I want to avoid ... > > > (as for why we still support mozldap - we still need an ldap c sdk that > supports NSS for crypto until we can fix that in the server. Once we > change 389 so that it can use openldap with openssl/gnutls for crypto, > we should consider deprecating support for mozldap.) Actually, all of this is specific to openldap only. Nothing about mozldap at all. So I think I'll just have to ignore the GCC warnings about this when it's related to openldap ber library. -- Sincerely, William Brown Software Engineer Red Hat, Brisbane signature.asc Description: This is a digitally signed message part -- 389-devel mailing list 389-devel@lists.fedoraproject.org https://lists.fedoraproject.org/admin/lists/389-devel@lists.fedoraproject.org
[389-devel] Re: Sign compare checking
On 08/28/2016 11:13 PM, William Brown wrote: So either, this is a bug in the way openldap uses the ber_len_t type, we have a mistake in our logic, or something else hokey is going on. I would like to update this to: if ( (tag != LBER_END_OF_SEQORSET) && (len == 0) && (*fstr != NULL) ) Or even: if ( (tag != LBER_END_OF_SEQORSET) && (*fstr != NULL) ) What do you think of this assessment given the ber_len_t type? Looks like it's intentional by the openldap team. There are some other areas for this problem. Specifically: int ber_printf(BerElement *ber, const char *fmt, ...); lber.h:79:#define LBER_ERROR((ber_tag_t) -1) We check if (ber_printf(...) != LBER_ERROR) Of course, we can't satisfy either. We can't cast the LBER_ERROR from uint -> int without changing the value of it, and we can't cast the output of ber_printf from int -> uint, again, without potentially changing the value of it. So it seems that the openldap library may be impossible to satisfy the gcc type checking with -Wsign-compare. For now, I may just avoid these in my fixes, as it seems like a whole set of landmines I want to avoid ... Part of the problem is that we wanted to support being able to use both mozldap and openldap, without too much "helper" code/macros/#ifdef MOZLDAP/etc. It looks as though this is a place where we need to have some sort of helper. (as for why we still support mozldap - we still need an ldap c sdk that supports NSS for crypto until we can fix that in the server. Once we change 389 so that it can use openldap with openssl/gnutls for crypto, we should consider deprecating support for mozldap.) -- 389-devel mailing list 389-devel@lists.fedoraproject.org https://lists.fedoraproject.org/admin/lists/389-devel@lists.fedoraproject.org -- 389-devel mailing list 389-devel@lists.fedoraproject.org https://lists.fedoraproject.org/admin/lists/389-devel@lists.fedoraproject.org
[389-devel] Re: Sign compare checking
> > So either, this is a bug in the way openldap uses the ber_len_t type, we > have a mistake in our logic, or something else hokey is going on. > > I would like to update this to: > > if ( (tag != LBER_END_OF_SEQORSET) && (len == 0) && (*fstr != NULL) ) > > Or even: > > if ( (tag != LBER_END_OF_SEQORSET) && (*fstr != NULL) ) > > What do you think of this assessment given the ber_len_t type? Looks like it's intentional by the openldap team. There are some other areas for this problem. Specifically: int ber_printf(BerElement *ber, const char *fmt, ...); lber.h:79:#define LBER_ERROR((ber_tag_t) -1) We check if (ber_printf(...) != LBER_ERROR) Of course, we can't satisfy either. We can't cast the LBER_ERROR from uint -> int without changing the value of it, and we can't cast the output of ber_printf from int -> uint, again, without potentially changing the value of it. So it seems that the openldap library may be impossible to satisfy the gcc type checking with -Wsign-compare. For now, I may just avoid these in my fixes, as it seems like a whole set of landmines I want to avoid ... -- Sincerely, William Brown Software Engineer Red Hat, Brisbane signature.asc Description: This is a digitally signed message part -- 389-devel mailing list 389-devel@lists.fedoraproject.org https://lists.fedoraproject.org/admin/lists/389-devel@lists.fedoraproject.org
