INN indentation

Julien ÉLIE julien at trigofacile.com
Sat Oct 16 12:24:50 UTC 2021


Hi Russ,

>> I see that HACKING recommends to include at the beginning of C files:
>> 
>>      #include "config.h"
>>      #include "clibrary.h"
>> 
>> Yet clibrary.h already includes config.h; I bet it had not always been the
>> case, which would explain it and why all or most of our files have both.
>> Maybe we could drop the inclusion of config.h then.  And globally review
>> the inclusions when clang-format re-orders them.
> 
> If we rename clibrary.h to portable/system.h, it will match the naming
> convention in rra-c-util where it largely comes from and will also avoid
> this problem.

I've had a look but wanted to be sure of what to do.
It seems like that all inclusions of "clibrary.h", or of both 
"clibrary.h" and "config.h", should be changed to only one inclusion of 
"portable/system.h".
And "clibrary.h" moved to "portable/system.h".


"portable/system.h" in rra-c-util uses:
#include <config.h>

"clibrary.h" uses:
#include "config.h"

Shouldn't the syntax with quotes be used?  ("config.h" is not a system 
header)



In HACKING:

   "clibrary.h" does also include "config.h", but it's somewhat poor form
   to rely on this; it's better to explicitly list the header
   dependencies for the benefit of someone else reading the code.

:)
We can get rid of that paragraph then.



Also, from TODO:

* The include files necessary to use libinn, libstorage, and other
   libraries should be installed in a suitable directory so that other
   programs can link against them.  All such include files should be under
   include/inn and included with <inn/header.h>.  All such include files
   should only depend on other inn/* header files and not on, e.g.,
   config.h.  All such include files should be careful about namespace to
   avoid conflicts with other include files used by applications.

Isn't it already done?
All headers in include/inn only include headers also in include/inn, 
which never include "config.h".
I think the namespace is ok (I'm not aware of any complaints about it).
Unless I am missing something, there isn't any work to do for that task, 
is it?




Finally, we already have "inn/system.h", with a few configure-time 
options set (like INN_HAVE_INET6 or INN_HAVE__BOOL) needed by INN headers.
It is notably included by "inn/defines.h" which only include 3 header 
files.  Time has come to get rid of "inn/defines.h" at the same time, 
and just include the necessary headers instead of it.


We would then have "portable/system.h" (the current "clibrary.h" which 
includes "config.h") along with "inn/system.h" (which does not include 
"config.h" like all headers in "inn").



>> AlignConsecutiveMacros: AcrossEmptyLinesAndComments
>> may be better but only available in clang-format 13 (not yet released!)
>> It think it would permit to have 62 at the same level here:
> 
>> #if defined(HAVE_OPENSSL) || defined(HAVE_SASL)
>> #    define PERMrequire_ssl 62
>> #    define PERMMAX         63
>> #else
>> #    define PERMMAX 62
>> #endif
> 
> Oh, that will be very nice once that's released.  That's one of the
> formatting decisions that always bugged me.

I also see that the next clang-format release (13) will permit aligning 
array of structs with for instance:
AlignArrayOfStructures: Right

struct test demo[] =
{
     {56,    23, "hello"},
     {-1, 93463, "world"},
     { 7,     5,    "!!"}
};

The current formatting breaks them badly...
I believe this new style option will permit removing a few /* 
clang-format off */ comments in the code.



> I'd be good to go with what's in rra-c-util right now, which is the one I
> sent earlier plus the enum change, and I think there was another vote for
> whatever as long as it's automated.

Adopted!

-- 
Julien ÉLIE

« – Ne te réjouis pas trop vite… Gnôthi seauton !
   – Ça veut dire quoi ça ?
   – Je ne sais pas ; c'est du grec. » (Oursenplus)


More information about the inn-workers mailing list