[PATCH 4/6] cli: intialize crypto structure in show and reply

Jani Nikula jani at nikula.org
Fri May 18 01:20:43 PDT 2012


*sigh* I'm failing to detach myself from this conversation. :(

On Thu, 17 May 2012, Daniel Kahn Gillmor <dkg at fifthhorseman.net> wrote:
> I don't think it's an assumption -- Jani is probably relying on the C
> standard. Consider, for example, C99 [0]'s section 6.7.8.19, which says:

Thanks for digging up the references.

> The real tradeoff in this choice is whether we prefer:
>
>  a) more compact code to facilitate quick reading by experts
>
>    or
>
>  b) more verbose code to facilitate comprehension by the non-expert.

We have -Wextra, which enables -Wmissing-field-initializers, which
requires us to use full initialization of struct fields when doing
regular, non-designated initialization. The point is that you might
introduce subtle bugs if you added new struct fields and forgot to check
the initializations. (This is why we have e.g. { 0, 0, 0, 0, 0 } instead
of just { 0 } in the initialization of notmuch_opt_desc_t arrays.)

IMHO the whole point of designated initializers is that the
initialization is not vulnerable to struct changes, and you can pick
which fields you choose to initialize explicitly. Also, it has the added
benefit of documenting the fields that are initialized, without having
to look at the struct definition.

Do we now want to initialize all struct fields explicitly, everywhere,
even when using designated initializers? Isn't that the question then?
Won't that maintain and promote the misconception that explicit
initialization is required, when it's really not, failing to educate the
non-experts and planting a seed of doubt in the experts...?

> I started this discussion leaning strongly toward the (b) perspective.
> But now that i know the relevant bits of the standard, i can sympathize
> with the (a) perspective as well :P

It's not always clear whether something is a matter of taste, style, or
language paradigm. If it feels like a paradigm, sticking with it
ultimately benefits *both* perspectives.

> Overall, i think i'm still in the (b) camp.  But i think it's more
> important that we don't allow dithering over this issue to prevent the
> inclusion of this patch series, which is a step in the right direction
> for handling S/MIME messages as well as PGP/MIME.

Agreed.

> PS gcc's -pedantic argument provides the following warning:
>
>  error: ISO C90 forbids specifying subobject to initialize
>
> So we probably want to specify -std=c99 at least to ensure our choice of
> subobject initialization is respected.

Unfortunately, the notmuch code base uses mixed standards, due to GCC
being so lax about it. Anything -pedantic produces warnings:
id:"cover.1325977940.git.jani at nikula.org". You may also want to try
clang to get better warnings.


BR,
Jani.


More information about the notmuch mailing list