From EHASZLA@transunion.com Mon Apr 5 05:52:44 2010 From: EHASZLA@transunion.com To: openldap-bugs@openldap.org Subject: RE: (ITS#6505) ldap_search_* functions missing const qualifiers Date: Mon, 05 Apr 2010 05:52:43 +0000 Message-ID: <201004050552.o355qhxn078313@boole.openldap.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7120984981281481593==" --===============7120984981281481593== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit This is a multi-part message in MIME format. ------_=_NextPart_001_01CAD483.E1DB174F Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable -----Original Message----- >From: masarati(a)aero.polimi.it [mailto:masarati(a)aero.polimi.it] >In general, upload to ftp.openldap.org according to contribution >instructions, and post a message with the URL; the way you provided it >makes it pretty unusable. I saw the instructions about using the ftp site some time after I sent = the patches. Sorry. The patch was just the obvious, mechanical change to add const to the attrs parameter in the functions that have it. Maybe a reminder to keep reading the rest of the page in the section on "patches" would be helpful. I got to the part that started talking = about attribution of large changes, and assumed the rest of the page wasn't relevant to what I was trying to do. >However, I think your patch will not be considered, although in = principle >perfectly acceptable, because an API change of this type would probably >break a lot of code out there, without significant advantages. Well, a couple of observations: 1) At least one other ldap implementation already does this. 2) I consider being able to turn on the compiler warnings that tell you = about parameters that might be changed when do don't expect them to be = a significant advantage. For many of the source trees I've worked on, = doing so (and treating warnings as errors, to force them to be fixed)=20 has helped uncover a variety of bugs. 3) If you're not being pedantic about warnings, and causing them to be=20 treated as errors, this API change just results in an extra warning,=20 which wouldn't actually break any code. 4) Adding "const" effectively and simply documents the fact that those=20 parameters aren't going to be changed. If the API doesn't say so, it would be nice if that was at least explicitly mentions in the man = page. At the moment, I had to go read an analyze the ldap code to figure that out. I'd rather spend my time working on my own code instead, = and be able to simply use the great library you guys have created. :) Anyway, if the change doesn't get accepted it won't be the end of the world, but obviously I'm hoping you find the above convincing. eric ------_=_NextPart_001_01CAD483.E1DB174F Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable
-----Original Message-----
>From: masarati(a)aero.polimi.it [mailto:masarati(a)aero.polimi.it]
>In general, upload to ftp.openldap.org according to contribution
>instructions, and post a message with the URL; the way you provided =
it
>makes it pretty unusable.
I saw the instructions about using the ftp site some time after I =
sent
the patches. Sorry. The patch was just the obvious, =
mechanical change
to add const to the attrs parameter in the functions that have it.
Maybe a reminder to keep reading the rest of the page in the section =
on
"patches" would be helpful. I got to the part that =
started talking about
attribution of large changes, and assumed the rest of the page =
wasn't
relevant to what I was trying to do.
>However, I think your patch will not be considered, although in =
principle
>perfectly acceptable, because an API change of this type would =
probably
>break a lot of code out there, without significant advantages.
Well, a couple of observations:
1) At least one other ldap implementation already does this.
2) I consider being able to turn on the compiler warnings that =
tell you
about parameters that might be changed when do don't expect them =
to be
a significant advantage. For many of the source trees I've =
worked on,
doing so (and treating warnings as errors, to force them to be =
fixed)
has helped uncover a variety of bugs.
3) If you're not being pedantic about warnings, and causing them =
to be
treated as errors, this API change just results in an extra =
warning,
which wouldn't actually break any code.
4) Adding "const" effectively and simply documents the =
fact that those
parameters aren't going to be changed. If the API doesn't =
say so, it
would be nice if that was at least explicitly mentions in the man =
page.
At the moment, I had to go read an analyze the ldap code to =
figure
that out. I'd rather spend my time working on my own code =
instead, and
be able to simply use the great library you guys have created. =
:)
Anyway, if the change doesn't get accepted it won't be the end of =
the
world, but obviously I'm hoping you find the above convincing.
eric