FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 02-25-2011, 03:27 PM
Dan McGee
 
Default Style questions and concerns

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.
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.
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.
* 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.
* 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.
* C99 structure initialization (saw it in the parallelize patches).

Please chip in and discuss.

-Dan
 
Old 02-26-2011, 12:50 AM
Allan McRae
 
Default Style questions and concerns

On 26/02/11 02:27, Dan McGee 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.


Seems reasonable.


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.


Agreed, a single patch post 3.5 would be good.


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.


I believe Cygwin's gcc is gcc3.4 by default, but they do provide gcc4
(4.5) packages. Has anyone attempted to compile pacman using gcc-3.x
lately?



* 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.


Agreed.


* 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.
* C99 structure initialization (saw it in the parallelize patches).


I'm not sure of the relative advantages/disadvantages of these so no
comment from me other than I doubt I would use C99 structure initialization.


Allan
 
Old 02-26-2011, 01:22 AM
Dave Reisner
 
Default Style questions and concerns

On Fri, Feb 25, 2011 at 10:27:13AM -0600, Dan McGee 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.

Agreed. If it doesn't need to be opaque, then there's no reason to force
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.

Really just comes down to to stylistic preference. It's not a style I
prefer personally, but it's also not my personal project. I'm happy to
follow whatever we agree upon.

> 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.
> * 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.
> * 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.
> * C99 structure initialization (saw it in the parallelize patches).
>

I see this as an all or none kind of deal. If we're going to make an
effort to avoid c99, I think we should avoid _all_ c99.

dave
 
Old 02-26-2011, 02:28 AM
Allan McRae
 
Default Style questions and concerns

On 26/02/11 11:50, Allan McRae wrote:

Has anyone attempted to compile pacman using gcc-3.x lately?


As an aside, current pacman master builds fine with gcc-3.4

Allan
 
Old 02-26-2011, 10:42 AM
Xavier Chantry
 
Default Style questions and concerns

On Fri, Feb 25, 2011 at 5:27 PM, Dan McGee <dpmcgee@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.

Currently we have a lot of the following :
private header : struct __foo_t { ... }
public header : typedef struct __foo_t foo_t

Of course that only makes sense to hide internal contents of public types.

In the other cases, we should just have :
typedef struct foo_t { ... } foo_t ;
and this can be either in a public or in a private header, depending
if it's a fully public or fully private struct.

IE I don't think the typedef is the problem here, the typedef is good
and avoids writing struct everywhere.

> 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.

I am confused. So all these features are c99, but gcc 3.x only has a
partial c99 support ?

> * * 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.

I agree, it sucks. What's cool is variable decl in loop.

> * * 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.
> * * C99 structure initialization (saw it in the parallelize patches).
>

Well we are in 2011, maybe it's time to start using features from the
previous Millennium.
At least it's still C for the nostalgic dinosaur.
 
Old 02-26-2011, 11:22 PM
Tavian Barnes
 
Default Style questions and concerns

On 25 February 2011 11:27, Dan McGee <dpmcgee@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
 

Thread Tools




All times are GMT. The time now is 09:50 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org