1

is safe if I use this macro in my code?

#define my_calloc(x, n) ((x) = (__typeof__ (x))calloc((n), sizeof(__typeof__ (&(x)))))

I'm usign gcc as compiler...

In my programm there is a lot of memory allocation point, so I use this. I tried it 5 minutes ago and I get some weird sigabort and sigsev, now I'm going home... after I'll try again if I can find something.

Some idea/tip?

EDIT ADDED:

Generally I use the macro as follows:

double *x;
my_calloc(x, 10);

int **y;
my_calloc(y, 30);
the_candyman
  • 1,563
  • 4
  • 22
  • 36
  • 4
    Note that it's unnecessary and potentially dangerous to cast the result of malloc/calloc in C. – Paul R Apr 05 '11 at 19:58
  • @Paul R: Is there any danger if you are compiling in C99, where there must be a declaration of `malloc()` in scope before it is used? – Jonathan Leffler Apr 05 '11 at 20:00
  • 1
    @Jonathan: you may be right - I can't think of a dangerous scenario when prototypes are mandatory. Even if we can assume C99 though, I never like to see unnecessary type casts - it's a "code smell". – Paul R Apr 05 '11 at 20:04
  • C99 does not mandate prototypes: `int main(void) { return abs(0); }` is a strictly conforming program. – pmg Apr 05 '11 at 20:19
  • @pmg: You are relying on an implicit declaration of `abs()` as a function returning `int`, and that is not permitted in strict C99. It would be OK with `extern int abs();` - a non-prototype declaration of the function. And, more relevantly, '`int main(void) { return malloc(10) != 0; }`' is not strictly conforming (because `malloc()` is not a function that returns an `int`; on M680x0 chips, the return value for `malloc()` should be in register A0, but because the compiler has been lied to, it would look in register D0 for the result, leading to problems!) – Jonathan Leffler Apr 05 '11 at 22:14

2 Answers2

9

I think it should probably be:

#define my_calloc(x, n) do { (x) = calloc((n), sizeof *(x)); } while (0)
  • unnecessary/dangerous cast has been removed
  • redundant parentheses removed
  • do/while added for correct behaviour between if (...) and else
  • fixed size of type
  • remove redundant and non-portable __typeof__
Paul R
  • 208,748
  • 37
  • 389
  • 560
  • +1, but you removed the *necessary* parentheses around the arguments `(x)` and `(n)` and left the useless ones after `sizeof`... It should read `#define my_calloc(x, n) do { (x) = calloc((n), sizeof *(x)) } while (0)` – R.. GitHub STOP HELPING ICE Apr 05 '11 at 20:08
  • @R. thanks - it's a (possibly bad) habit of mine to always put parentheses after `sizeof`, probably because I can never remember the rule for when they are required and when not. ;-) – Paul R Apr 05 '11 at 20:10
  • @R.: why does `x` need parentheses ? It can only ever be a variable name, surely ? – Paul R Apr 05 '11 at 20:12
  • They're only needed for grouping or if you're applying `sizeof` to a parenthesized *type name* and not an expression. – R.. GitHub STOP HELPING ICE Apr 05 '11 at 20:13
  • `x` could be any lvalue of pointer type. Of course if you find that repulsive you could document that `x` must be simply a variable name, but surely someone will want to use it with slightly more complex lvalues like perhaps `*dblptr`. – R.. GitHub STOP HELPING ICE Apr 05 '11 at 20:13
  • @Paul: why the use of a while loop? – the_candyman Apr 05 '11 at 20:14
  • @the_candyman: see answer for explanation - you need do/while (0) for correct behaviour in a situation such as `if (...) my_calloc(x, n); else ...` – Paul R Apr 05 '11 at 20:16
  • @Paul R, you are missing one of the advantages in your list: avoid useless use of the `__typeof__` extension. – Jens Gustedt Apr 05 '11 at 20:23
  • @Paul: I don't think the `do/while` is strictly necessary here. The `(x) = ...` expression won't confuse an `if` statement (though the expression should be wrapped in parens in that case, as should all macros that evaluate to expressions). However, I think there's no harm in using the `do/while` wrapper, since `my_calloc()` is unlikely to be used as a sub-expression. – Michael Burr Apr 06 '11 at 00:38
  • @Michael: yes, I think you may be right in this instance - I usually take a a defensive approach in these matters - it's safer in general and it also saves me form having to think too much. ;-) – Paul R Apr 06 '11 at 06:44
  • @Paul: "saves me from having to think too much" - a damn good reason for using an idiom. – Michael Burr Apr 06 '11 at 13:44
  • @Paul. I tried the proposed alternative macro, but I get always error... I don't get error if I use the ";"...#define my_calloc(x, n) do { (x) = calloc((n), sizeof *(x)) ; } while (0) – the_candyman Apr 07 '11 at 07:16
  • 1
    @the_candyman: sorry about the missing `;` - answer edited to include this now. – Paul R Apr 07 '11 at 08:14
4

Your macro allocates n pointers not objects. Try sizeof(*(x)).

Ben Jackson
  • 90,079
  • 9
  • 98
  • 150