3

Is it bad practice to allocate memory like this?:

FOO *foo;
while (!(foo = malloc(sizeof(FOO)))) ;
Matt
  • 21,026
  • 18
  • 63
  • 115
  • Why would `malloc` suddenly succeed when you try again after it failed once? – Daniel Fischer Jan 17 '13 at 17:05
  • 1
    It looks hinky. For one thing, it has the expectation that the malloc will eventually succeed. Maybe that's a reasonable expectation, maybe not. – Robert Harvey Jan 17 '13 at 17:05
  • Probably. You could cause a race condition. Maybe limit it a certain amount of times, or write your own malloc function which calls malloc and tries only 100 times. – Kirk Backus Jan 17 '13 at 17:05
  • 1
    So essentially if your system is unable to allocate memory, now it will also be running low on CPU cycles? – JasonD Jan 17 '13 at 17:06
  • If your system fails to allocate memory, you should probably just end the program altogether. – Kirk Backus Jan 17 '13 at 17:06
  • As this is a style question, it would be better on either [Programmers.SE] or [Codereview.SE] – Kevin Jan 17 '13 at 17:06
  • And I'd say it's fine, but you should use `sizeof(*foo)` instead of `sizeof(FOO)` – Kevin Jan 17 '13 at 17:07
  • @Kevin, that's kind of a value judgement. Why not `sizeof *foo`? That is, why the extra `()` if you're going to use the variable rather than the type? – Carl Norum Jan 17 '13 at 17:08
  • @CarlNorum My comment was purely on the `*foo` vs `FOO`, no judgement on the parens. `*foo` is better because if you change `foo`'s type, you might forget to update or miss a `sizeof`; `FOO` may then allocate the wrong size, but you needn't go update the `*foo`s. – Kevin Jan 17 '13 at 17:17
  • @Kevin, I agree with you in most cases, but if the declaration of `foo` is far from the `malloc` call, I've found it helpful to have the type version, as a reminder of how much memory is getting allocated. Maybe less of an issue with an aggregate type, though. – Carl Norum Jan 17 '13 at 17:18
  • @DanielFischer - 'cos some other thread releases enough memory. Still, I would not do this even on embedded RAM-poor systems. Like the other posters have said, a malloc fail is normally deadly, sooner or later. – Martin James Jan 17 '13 at 18:44
  • @MartinJames Yes, it's possible, if you loop long enough. But a busy loop screams "I expect to get a success really soon". Now, if it had been `while(!(foo = malloc(whatever))) { sleep(1); }`, that would kind of make sense. – Daniel Fischer Jan 17 '13 at 18:48

2 Answers2

4

I don't know about bad practice, but it's uncommon. malloc() failures are usually indicative of major system problems that your program is unlikely to be able to recover from. If your system is different, your example may very well be practical.

NB - this answer assumes that sizeof(FOO) is "reasonable" and that your malloc() doesn't just refuse because you're asking for too much memory.

Carl Norum
  • 219,201
  • 40
  • 422
  • 469
1

That doesn't "guarantee" a result from malloc(). If malloc returns NULL there's probably a reason for it (out of memory for example). And you might throw yourself into an infinite loop.

Furthermore, if you're running this on a Linux platform:

By default, Linux follows an optimistic memory allocation strategy. This means that when malloc() returns non-NULL there is no guarantee that the memory really is available.

That means that lets say calling malloc over and over again did happen to return something non-NULL, due to Linux's "optimistic" strategy, a non-NULL still doesn't guarantee you got anything that will work.

I think this code is setting you up for a debugging nightmare.

Mike
  • 47,263
  • 29
  • 113
  • 177