--f46d04426a60f30297050c76e815 Content-Type: text/plain; charset=UTF-8
2015-01-12 17:19 GMT+03:00 Hallvard Breien Furuseth < h.b.furuseth@usit.uio.no>:
On 01/06/2015 04:31 PM, hyc@symas.com wrote:
Leonid Yuriev wrote:
Yes of course, all warnings could be hushed by compiler options, but this also hides the real mistakes. So, I prefer fix the warnings, instead of disable gcc/clang features.
Please review attached patchset and merge.
Your patch assumes CPP features that are not guaranteed to be present. Rejecting this patchset.
The Debug() change is going to give merge conflicts all over the place when your https://github.com/ReOpen/ReOpenLDAP and OpenLDAP pull updates from each other.
https://github.com/ReOpen/ReOpenLDAP should not be used for merge, but
only for cherry-picking. It has stripped history and merged with OpenLDAP/mdb.master, because mdb.RE/0.9 don't includes a few critical fixes (ITS 7969,7970,7971) for our case. I think that is the reason for patch conflicts. Patchset got RE25 should remain among my sent emails. Presumably I can send it for you again, if you are interested.
How about defining Debug1(), Debug2(), etc for 1,2... arguments? That's still annoying, but it keeps the code C90-compatible and keeps -Wformat quiet. There's already similar code for Log<N>() in include/ldap_log.h, but it was never used much or even merged into RE24. (Because it got scheduled for RE25 which was going to start Real Soon Now, IIRC:-)
-- Hallvard
IMHO the Log<N> recipe is onerously for support. On other hand, nowadays all non-obsolete compilers support most of C99 features. Baseline features, such as "variadic macro", supported long ago (Microsoft was the most conservative to 2012). Maybe it is time to enable C99 in RE25?
Leonid.
--f46d04426a60f30297050c76e815 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
<div dir=3D"ltr"><div><div><br></div></div><div class=3D"gmail_extra"><div = class=3D"gmail_quote">2015-01-12 17:19 GMT+03:00 Hallvard Breien Furuseth <= span dir=3D"ltr"><<a href=3D"mailto:h.b.furuseth@usit.uio.no" target=3D"= _blank">h.b.furuseth@usit.uio.no</a>></span>:<br><blockquote class=3D"gm= ail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-l= eft-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span = class=3D"">On 01/06/2015 04:31 PM, <a href=3D"mailto:hyc@symas.com" target= =3D"_blank">hyc@symas.com</a> wrote:<br> <blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-= left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;p= adding-left:1ex"> Leonid Yuriev wrote:<br> <blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-= left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;p= adding-left:1ex"> Yes of course, all warnings could be hushed by compiler options, but<br> this also hides the real mistakes.<br> So, I prefer fix the warnings, instead of disable gcc/clang features.<br> <br> Please review attached patchset and merge.<br> </blockquote> <br> Your patch assumes CPP features that are not guaranteed to be present.<br> Rejecting this patchset.<br> </blockquote> <br></span> The Debug() change is going to give merge conflicts all over<br> the place when your <<a href=3D"https://github.com/ReOpen/ReOpenLDAP" ta= rget=3D"_blank">https://github.com/ReOpen/<u></u>ReOpenLDAP</a>> and<br> OpenLDAP pull updates from each other.<br> <br></blockquote><div><a href=3D"https://github.com/ReOpen/ReOpenLDAP%22%3Ehttp= s://github.com/ReOpen/ReOpenLDAP</a> should not be used for merge, but only= for cherry-picking.</div><div>It has stripped history and merged with Open= LDAP/mdb.master, because mdb.RE/0.9 don't includes a few critical fixes= (ITS 7969,7970,7971) for our case.</div><div>I think that is the reason fo= r patch conflicts.</div><div>Patchset got RE25 should remain among my sent = emails. Presumably I can send it for you again, if you are interested.</div=
<div>=C2=A0<br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px=
0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);bor= der-left-style:solid;padding-left:1ex"> How about defining Debug1(), Debug2(), etc for 1,2... arguments?<br> That's still annoying, but it keeps the code C90-compatible and<br> keeps -Wformat quiet.=C2=A0 There's already similar code for Log<N&g= t;()<br> in include/ldap_log.h, but it was never used much or even merged<br> into RE24.=C2=A0 (Because it got scheduled for RE25 which was going to<br> start Real Soon Now, IIRC:-)<span class=3D""><font color=3D"#888888"><br> <br> -- <br> Hallvard<br> </font></span></blockquote></div><br></div><div class=3D"gmail_extra"><div = class=3D"gmail_extra"><div class=3D"gmail_extra">IMHO the Log<N> reci= pe is onerously for support.</div><div class=3D"gmail_extra">On other hand,= nowadays all non-obsolete compilers support most of C99 features.</div><di= v class=3D"gmail_extra">Baseline features, such as "variadic macro&quo= t;, supported long ago (Microsoft was the most conservative to 2012).</div>= <div class=3D"gmail_extra">Maybe it is time to enable C99 in RE25?<br></div=
<div class=3D"gmail_extra"><br></div><div class=3D"gmail_extra">Leonid.</d=
iv></div></div></div>
--f46d04426a60f30297050c76e815--
-
leo＠yuriev.ru