[pacman-dev] Style questions and concerns

[Tavian Barnes]

On 25 February 2011 11:27, Dan McGee <dpmcgee at gmail.com> wrote:
> I've noticed these things a lot in recent patches, so let the
> discussion commence.
>
> 1) typedef-ed structs. This is just a "copy the rest of them" habit,
> but I really feel we should stop doing this, only typedef-ing when we
> are making a public-facing type that we don't want to expose the
> internal contents of. Everything else should remain a struct type, and
> referred to as such, no typedef and no confusion that the fields are
> accessible.

At least in the case of my patches, this is a personal habit of mine
that probably stems from learning C++ first a long time ago, so
"struct foo" seems verbose and unnecessary to me.  But if this is the
established coding style I'm happy to follow it.

> 2) return(value) style. This is a rule set in stone in the coding
> style doc but people are continuously breaking it. To be fair, it is
> silly, but mixing styles sucks way worse. I propose switching this,
> *BUT* all current patches need to follow current style, and if we
> switch, it should happen post-3.5.0 and in one commit, not this
> piecemeal junk.

This semantic patch should cure them all at once if you want to switch:

@@
expression expr;
@@

return
-(
expr
-)
;

> 3) C99. Several patches are introducing things that go further down
> this road. I don't think they should all be avoided but I'm not sure
> GCC 3.X (Cygwin is still on this?) supports all of these.

Plenty of C99 features have been GCC C extensions for a long time,
such as mixed declarations and code, and variable-sized arrays.

>   * Variable declarations not at the start of a block. I'm not a huge
> fan of this, as I think having them at the start of the block helps
> clarify scope a lot better.

Another C++ habit, but I really like it in C too.  Especially in for
loops, and extra especially when you want to iterate in two different
ways in one function (linked list and array for example, which
requires two different types for `i').

>   * Dynamically sized array declarations, e.g. data[numcpus]. I think
> this needs to be avoided, unfortunately, but it isn't hard to just use
> MALLOC/FREE in this case.

Fair enough.

>   * C99 structure initialization (saw it in the parallelize patches).

Already in the codebase, for example the definition of default_pkg_ops
in lib/libalpm/package.c.

> Please chip in and discuss.
>
> -Dan

-- 
Tavian Barnes