[Freeciv-Dev] Re: (PR#11779) New genlist code
[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
<URL: http://bugs.freeciv.org/Ticket/Display.html?id=11779 >
On Sat, 29 Jan 2005, Jason Short wrote:
> <URL: http://bugs.freeciv.org/Ticket/Display.html?id=11779 >
>
> - When updating the copyright, I think it's wrong to remove the old
> year. "Copyright 1996" becomes "Copyright 1996-2005" or maybe
> "Copyright 1996,2005". BTW, the year is 2005 now ;-).
'Copyright 1996-2005' or 'Copyright 1996..2005'. The comma doesn't make it
obvious that there was intermediate work.
> - For function headers that don't take any paraneters, put (void) in as
> the parameter. This is only really important for prototypes (since an
> "empty" list will match any function parameters (a misfeature of C I
> guess) but can save some confusion for the headers of function bodies as
> well.
Yes, this isn't C++. Into the 'void' we must go. :-)
> - Tell me again why genlist_free doesn't call genlist_unlink_all?
> Having an error message or assertion here might be helpful but I can't
> think of any reason why you'd want to leak memory on purpose.
Agreed. I keep forgetting to do it. Adding this would make code less
memory leak prone and have less tedious typing involved.
> - In mallocs it's better to use the variable rather than the type in the
> sizeof. E.g., instead of
>
> struct genlist_link *new_link
> = fc_malloc(sizeof(struct genlist_link));
>
> use sizeof(*new_link). Many's the time I've changed a variable type and
> had to worry about updating the malloc calls as well - annoying and
> avoidable.
Indeed. I always use this form because of that. It also makes more sense
really.
> - Avoidable casts are to be avoided. E.g.
>
> && ((struct genlist *)(plink->dataptr))->ndeleted > 0) {
>
> can use a tmp. variable and avoid a cast.
>
> Yeah yeah, I think I mentioned some of these before.
Yes, I also dislike sticking casts in the middle of expressions, most of
the time.
---
Vasco Alexandre da Silva Costa @ Instituto Superior Tecnico, Lisboa
|
|