Step 1: Move the declaration of stack
inside main
. There's no reason it should be declared globally:
int main( void )
{
NODEPTR *stack;
...
Step 2: Move the malloc
call inside main
(you cannot perform an assignment or a function call outside of a function).
Step 3: Drop the cast; it's unnecessary1 and just adds visual clutter.
Step 4: Use sizeof *stack
as opposed to sizeof (NODEPTR)
in the argument to malloc
:
stack = malloc( sizeof *stack * 20 );
The result is the same, but this is easier to read, and avoids maintenance headaches if you ever change the type of stack
.
Step 5: free
your stack when you're done. Yeah, for this program it doesn't matter, but it's a good habit to get into.
So after all this, your code should read:
int main( void )
{
NODEPTR *stack;
stack = malloc( sizeof *stack * 20 );
...
free( stack );
return 0;
}
Stylistic nit: Hiding pointer types behind typedefs is bad juju IME. Pointer semantics are important, and if the programmer is ever expected to dereference an object of type NODEPTR
(either explicitly, as (*node).info
, or implicitly, as node->info
), then it's usually best to declare that object using pointer declaration syntax, something like
typedef struct treenode Node;
Node *node;
Node **stack;
...
stack[i]->next = node->next;
etc. so the person using your types knows exactly how many levels of indirection are involved and can write their code accordingly (multiple indirection is not hard). If the type is meant to be truly opaque and never directly dereferenced, but just passed around to an API that handles all that, then hiding the pointerness of that type is okay. Otherwise, leave it exposed.
I tend not to typedef struct types for a similar reason, but I suspect I'm an outlier in that regard.
Ahd who broke the code formatter?!
1 - In C, that is; in C++, the cast is required, but if you're writing C++, you should be using new
instead of malloc
anyway.