[Date Prev][Date Next]
[Chronological]
[Thread]
[Top]
RE: (ITS#6505) ldap_search_* functions missing const qualifiers
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@aero.polimi.it [mailto:masarati@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
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<META HTTP-EQUIV=3D"Content-Type" CONTENT=3D"text/html; =
charset=3Diso-8859-1">
<META NAME=3D"Generator" CONTENT=3D"MS Exchange Server version =
6.5.7654.12">
<TITLE>RE: (ITS#6505) ldap_search_* functions missing const =
qualifiers</TITLE>
</HEAD>
<BODY>
<!-- Converted from text/plain format -->
<P><FONT SIZE=3D2>-----Original Message-----<BR>
>From: masarati@aero.polimi.it [<A =
HREF=3D"mailto:masarati@aero.polimi.it">mailto:masarati@aero.polimi.it</A=
>]<BR>
>In general, upload to ftp.openldap.org according to contribution<BR>
>instructions, and post a message with the URL; the way you provided =
it<BR>
>makes it pretty unusable.<BR>
I saw the instructions about using the ftp site some time after I =
sent<BR>
the patches. Sorry. The patch was just the obvious, =
mechanical change<BR>
to add const to the attrs parameter in the functions that have it.<BR>
<BR>
Maybe a reminder to keep reading the rest of the page in the section =
on<BR>
"patches" would be helpful. I got to the part that =
started talking about<BR>
attribution of large changes, and assumed the rest of the page =
wasn't<BR>
relevant to what I was trying to do.<BR>
<BR>
>However, I think your patch will not be considered, although in =
principle<BR>
>perfectly acceptable, because an API change of this type would =
probably<BR>
>break a lot of code out there, without significant advantages.<BR>
<BR>
Well, a couple of observations:<BR>
1) At least one other ldap implementation already does this.<BR>
2) I consider being able to turn on the compiler warnings that =
tell you<BR>
about parameters that might be changed when do don't expect them =
to be<BR>
a significant advantage. For many of the source trees I've =
worked on,<BR>
doing so (and treating warnings as errors, to force them to be =
fixed)<BR>
has helped uncover a variety of bugs.<BR>
3) If you're not being pedantic about warnings, and causing them =
to be<BR>
treated as errors, this API change just results in an extra =
warning,<BR>
which wouldn't actually break any code.<BR>
4) Adding "const" effectively and simply documents the =
fact that those<BR>
parameters aren't going to be changed. If the API doesn't =
say so, it<BR>
would be nice if that was at least explicitly mentions in the man =
page.<BR>
At the moment, I had to go read an analyze the ldap code to =
figure<BR>
that out. I'd rather spend my time working on my own code =
instead, and<BR>
be able to simply use the great library you guys have created. =
:)<BR>
<BR>
Anyway, if the change doesn't get accepted it won't be the end of =
the<BR>
world, but obviously I'm hoping you find the above convincing.<BR>
<BR>
eric</FONT>
</P>
</BODY>
</HTML>
------_=_NextPart_001_01CAD483.E1DB174F--