Hot answers tagged

17

p = malloc(n*sizeof *p); This is dangerous if n gets large, because the multiplication could overflow. After the overflow, too little memory has been allocated but code will continue without detecting the error. This is especially dangerous if n comes from untrusted source, such as some file format or remote user. Then it gives attacker easy way to ...


8

You've taken an expression statement and unnecessarily made a do/while statement out of it. You need parentheses around your macro parameters. You don't need to pass in the pointer you're assigning to as a pointer. Put all that together and you end up with: #define ALLOC(p, n) ((p) = malloc((n) * sizeof *(p))) This puts fewer restrictions on how it is ...


5

Most severe issue: Never invent secret macro languages! This is about the single-worst thing a C programmer can ever do, all categories. You are perfectly free to assume that any C programmer will understand what 1<<bit means. You cannot assume that they will understand what BFLAGS_WORD_MASK means. It's nothing but obfuscation of what would ...


5

Small portability bug: if we're using false in the macros, then bflags.h should include <stdbool.h>. Or use 0 instead of false. Definite portability bug: we shift a (signed) int before promoting: #define BFLAGS_WORD_MASK(bit) (1 << ((bit) % BFLAGS_WORD_BIT_SIZE)) I think we need the 1 there to be of the appropriate type first: #define ...


4

There are a couple questionable things here, first since ALLOC() is a macro you don't need to pass the address of p. If ALLOC() were a function call then you would want to pass the address of p. Second, for an array I would probably use calloc() rather than malloc().


2

I have only questions, but maybe some of these are useful. It is desirable to fail fast and in a predictable manner to improper LINKEDLIST_GENERATE commands, maybe with a comment saying what's wrong. LINKEDLIST_GENERATE_HEADER_PUBLIC, etc is probably too confusing; could it could be simplified? You include FMOD; these defines appear interconnected. owner ...


2

Based on your question, a similar question I asked yesterday, and some of the answers, I did a version that tries to have the benefits of all of them. Disclaimer: Code is GCC specific (although probably works in similar compilers such as Clang). Usage: int *p; /* Want >int p[47];< */ if (mallocs(&p, 47)) goto err; ... free(p); ...


2

First of all, remove the unnecessary parts, which can cause errors and make the code harder to read. #define ALLOC(p,n) p=malloc(n*sizeof *p) You can make your code more readable with describing names. After months it will be quite hard to understand even your own code. #define ALLOC(pointer, size) pointer = malloc(size * sizeof *pointer) Parenthesizes ...


2

Pedantically, if (snprintf(buff, sizeof(buff), format, ...) >= SSIZEOF(buff)) goto err; is an insufficient test. Test for < 0 is also needed. The snprintf function returns the number of characters that would have been written had n been sufficiently large, not counting the terminating null character, or a negative value if an encoding error ...


1

This is a good and useful idea. The name is intuitive and memorable. My first thought was that if a pointer were passed, then we'd use the size of the pointer, but you've found a good way to ensure only arrays are passed. Obviously, this means that we can't use this for those occasions where we build up the content in pieces (with several prints into a ...


Only top voted, non community-wiki answers of a minimum length are eligible