Re: [PATCH 5/8] Test for number of mkdir() arguments
Pawel Kot <[email protected]> Tue, 4 Dec 2018 00:24:14 +0300
| Newsgroups | gmane.linux.drivers.gnokii |
|---|---|
| Message-ID | <CAHytCw7C1Zz8E7NAC4W155bP+NoDyrMv3BZGpX4gV3A=uPiwqw@mail.gmail.com> |
--===============1115802752536070136== Content-Type: multipart/alternative; boundary="000000000000fc86ea057c24c563" --000000000000fc86ea057c24c563 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Ladis, On Mon, Dec 3, 2018 at 11:28 PM Ladislav Michl <[email protected]> wrote= : > there's an issue with changes like that one bellow. > Currently each file including compat.h have to include > config.h before on its own. That does not sound too > convenient and is quite error prone as I just found > testing mkdir patch on more configurations. I remeber it being discussed. Still not quite sure why we did it that way. I believe #include "config.h" from compat.h should be safe. Even without removing all others. I believe that whenever you need something from "config.h" you should explicitly include it. Cheers, Pawe=C5=82 > So what about this change: > > diff --git a/include/compat.h b/include/compat.h > index 8711c425..60974344 100644 > --- a/include/compat.h > +++ b/include/compat.h > @@ -18,6 +18,17 @@ > #ifndef _gnokii_compat_h > #define _gnokii_compat_h > > +#if defined(HAVE_CONFIG_H) > +# include "config.h" > + > +#elif defined(WIN32) && defined(_MSC_VER) > +# include "windows-config.h" > + > +#else > +# error "compat.h requires config.h" > + > +#endif > + > #include <stdlib.h> > #include <stdio.h> > > Perhaps ommiting windows-config.h part in the first round. > Then we could delete config.h include from each file which > also include compat.h. That is... almost each file. > > Objections? > > > On Mon, Dec 03, 2018 at 10:42:06AM +0100, Ladislav Michl wrote: > > Windows' mkdir() takes only 1 argument, so add a test for that. > > --- > > configure.ac | 1 + > > include/compat.h | 15 ++++++++++++++- > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/configure.ac b/configure.ac > > index 379c7412..cd2064dd 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -902,6 +902,7 @@ AC_PROG_GCC_TRADITIONAL > > AC_FUNC_MEMCMP > > AC_TYPE_SIGNAL > > AC_FUNC_STRFTIME > > +AC_FUNC_MKDIR > > AC_CHECK_FUNCS(mktime timegm gettimeofday select poll wcrtomb) > > AC_CHECK_FUNCS(strchr strdup strndup strstr strtol strtok strsep) > > AC_CHECK_FUNCS(asprintf vasprintf snprintf vsnprintf getpass setenv) > > diff --git a/include/compat.h b/include/compat.h > > index 009f5f0e..8711c425 100644 > > --- a/include/compat.h > > +++ b/include/compat.h > > @@ -199,6 +199,20 @@ int vasprintf(char **ptr, const char *format, va_list ap); > > time_t timegm(struct tm *tm); > > #endif > > > > +#if HAVE_MKDIR > > +# if MKDIR_TAKES_ONE_ARG > > + /* Mingw32 */ > > +# define mkdir(a,b) mkdir(a) > > +# endif > > +#else > > +# if HAVE__MKDIR > > + /* plain Win32 */ > > +# define mkdir(a,b) _mkdir(a) > > +# else > > +# error "Don't know how to create a directory on this system." > > +# endif > > +#endif > > + > > /* > > * The following code was taken from W. Richard Stevens' > > * "UNIX Network Programming", Volume 1, Second Edition. > > @@ -219,7 +233,6 @@ time_t timegm(struct tm *tm); > > #ifdef WIN32 > > # ifdef _MSC_VER > > # define inline __inline > > -# define mkdir(dirname, accessrights) _mkdir(dirname) > > # define strcasecmp _stricmp > > # define strncasecmp _strnicmp > > # define __const const > > -- > > 2.20.0.rc1 > > > > > > _______________________________________________ > > gnokii-users mailing list > > [email protected] > > https://lists.nongnu.org/mailman/listinfo/gnokii-users > > _______________________________________________ > gnokii-users mailing list > [email protected] > https://lists.nongnu.org/mailman/listinfo/gnokii-users -- Pawel Kot --000000000000fc86ea057c24c563 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"ltr">Hi Ladis,<br><br>On Mon, Dec 3, 2018 at 11:28 PM Ladislav = Michl <<a href=3D"mailto:[email protected]">[email protected]</a>&= gt; wrote:<br>> there's an issue with changes like that one bellow.<= br>> Currently each file including compat.h have to include<br>> conf= ig.h before on its own. That does not sound too<br>> convenient and is q= uite error prone as I just found<br>> testing mkdir patch on more config= urations.<br><br>I remeber it being discussed. Still not quite sure why we = did it that way.<div>I believe #include "config.h" from compat.h = should be safe. Even without removing all others. I believe that whenever y= ou need something from "config.h" you should explicitly include i= t.</div><div><br></div><div>Cheers,</div><div>Pawe=C5=82</div><div><br>>= So what about this change:<br>><br>> diff --git a/include/compat.h b= /include/compat.h<br>> index 8711c425..60974344 100644<br>> --- a/inc= lude/compat.h<br>> +++ b/include/compat.h<br>> @@ -18,6 +18,17 @@<br>= > =C2=A0#ifndef =C2=A0 =C2=A0 =C2=A0 =C2=A0_gnokii_compat_h<br>> =C2= =A0#define =C2=A0 =C2=A0 =C2=A0 =C2=A0_gnokii_compat_h<br>><br>> +#if= defined(HAVE_CONFIG_H)<br>> +# =C2=A0include "config.h"<br>&g= t; +<br>> +#elif defined(WIN32) && defined(_MSC_VER)<br>> +# = =C2=A0include "windows-config.h"<br>> +<br>> +#else<br>>= +# =C2=A0error "compat.h requires config.h"<br>> +<br>> +#= endif<br>> +<br>> =C2=A0#include <stdlib.h><br>> =C2=A0#incl= ude <stdio.h><br>><br>> Perhaps ommiting windows-config.h part = in the first round.<br>> Then we could delete config.h include from each= file which<br>> also include compat.h. That is... almost each file.<br>= ><br>> Objections?<br>><br>><br>> On Mon, Dec 03, 2018 at 10= :42:06AM +0100, Ladislav Michl wrote:<br>> > Windows' mkdir() tak= es only 1 argument, so add a test for that.<br>> > ---<br>> > = =C2=A0<a href=3D"http://configure.ac">configure.ac</a> =C2=A0 =C2=A0 | =C2= =A01 +<br>> > =C2=A0include/compat.h | 15 ++++++++++++++-<br>> >= ; =C2=A02 files changed, 15 insertions(+), 1 deletion(-)<br>> ><br>&g= t; > diff --git a/<a href=3D"http://configure.ac">configure.ac</a> b/<a = href=3D"http://configure.ac">configure.ac</a><br>> > index 379c7412..= cd2064dd 100644<br>> > --- a/<a href=3D"http://configure.ac">configur= e.ac</a><br>> > +++ b/<a href=3D"http://configure.ac">configure.ac</a= ><br>> > @@ -902,6 +902,7 @@ AC_PROG_GCC_TRADITIONAL<br>> > =C2= =A0AC_FUNC_MEMCMP<br>> > =C2=A0AC_TYPE_SIGNAL<br>> > =C2=A0AC_F= UNC_STRFTIME<br>> > +AC_FUNC_MKDIR<br>> > =C2=A0AC_CHECK_FUNCS(= mktime timegm gettimeofday select poll wcrtomb)<br>> > =C2=A0AC_CHECK= _FUNCS(strchr strdup strndup strstr strtol strtok strsep)<br>> > =C2= =A0AC_CHECK_FUNCS(asprintf vasprintf snprintf vsnprintf getpass setenv)<br>= > > diff --git a/include/compat.h b/include/compat.h<br>> > ind= ex 009f5f0e..8711c425 100644<br>> > --- a/include/compat.h<br>> &g= t; +++ b/include/compat.h<br>> > @@ -199,6 +199,20 @@ int vasprintf(c= har **ptr, const char *format, va_list ap);<br>> > =C2=A0time_t timeg= m(struct tm *tm);<br>> > =C2=A0#endif<br>> > <br>> > +#if= HAVE_MKDIR<br>> > +# if MKDIR_TAKES_ONE_ARG<br>> > + =C2=A0 /*= Mingw32 */<br>> > +# =C2=A0define mkdir(a,b) mkdir(a)<br>> > += # endif<br>> > +#else<br>> > +# if HAVE__MKDIR<br>> > + = =C2=A0 /* plain Win32 */<br>> > +# =C2=A0define mkdir(a,b) _mkdir(a)<= br>> > +# else<br>> > +# =C2=A0error "Don't know how t= o create a directory on this system."<br>> > +# endif<br>> &g= t; +#endif<br>> > +<br>> > =C2=A0/*<br>> > =C2=A0 * The f= ollowing code was taken from W. Richard Stevens'<br>> > =C2=A0 * = "UNIX Network Programming", Volume 1, Second Edition.<br>> >= ; @@ -219,7 +233,6 @@ time_t timegm(struct tm *tm);<br>> > =C2=A0#ifd= ef WIN32<br>> > =C2=A0# =C2=A0ifdef _MSC_VER<br>> > =C2=A0# =C2= =A0 =C2=A0define inline __inline<br>> > -# =C2=A0 =C2=A0define mkdir(= dirname, accessrights) _mkdir(dirname)<br>> > =C2=A0# =C2=A0 =C2=A0de= fine strcasecmp _stricmp<br>> > =C2=A0# =C2=A0 =C2=A0define strncasec= mp _strnicmp<br>> > =C2=A0# =C2=A0 =C2=A0define __const const<br>>= > --<br>> > 2.20.0.rc1<br>> ><br>> ><br>> > ___= ____________________________________________<br>> > gnokii-users mail= ing list<br>> > <a href=3D"mailto:[email protected]">gnokii-use= [email protected]</a><br>> > <a href=3D"https://lists.nongnu.org/mailman/= listinfo/gnokii-users">https://lists.nongnu.org/mailman/listinfo/gnokii-use= rs</a><br>><br>> _______________________________________________<br>&= gt; gnokii-users mailing list<br>> <a href=3D"mailto:gnokii-users@nongnu= .org">[email protected]</a><br>> <a href=3D"https://lists.nongnu.o= rg/mailman/listinfo/gnokii-users">https://lists.nongnu.org/mailman/listinfo= /gnokii-users</a><br><br><br><br>--<br>Pawel Kot<br></div></div> --000000000000fc86ea057c24c563-- --===============1115802752536070136== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ gnokii-users mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/gnokii-users --===============1115802752536070136==--