Eyzuky Eyzuky - 3 months ago 11
C Question

Pointer to struct, trying to refer to its data members - C

I have this code:

typedef struct node
{
char* data;
struct node next;
} node;

typedef struct linkedList
{
node head;
int size;
} linkedList;

typedef struct _MyLinkedList *MyLinkedListP
{
struct linkedList* listp;
};

MyLinkedListP createList()
{
linkedList *list = (linkedList*)malloc(sizeof(linkedList));
(*list).head = NULL;
(*list).size = 0;
MyLinkedListP listP = list;
return listP;
}


I am trying to build a function:

MyLinkedListP cloneList(MyLinkedListP l)


l
is a pointer to a
linkedList
, I am trying to get its members (size and first node), in order to complete the function. But I just do not understand how to do it under this build.
I also want to ask regarding my build of struct
MyLinkedListP
. My goal was to create a type that is a pointer to a
linkedList
, is the way I did it good practice?

Answer

First of all, this won't work:

typedef struct node
{
  char* data;
  struct node next;
} node;

A struct type may not contain a member that's an instance of itself1; this should be written as

typedef struct node
{
  char* data;
  struct node *next;  // next is a *pointer* to struct node
} node;

Secondly, while the following isn't a problem like the above, this is not good practice:

typedef struct _MyLinkedList *MyLinkedListP

Identifiers with leading underscores are reserved for the implementation; you should not create any type or object names with leading underscores. Just use

typedef struct MyLinkedList *MyLinkedListP

instead2.

Third, lose the cast in

linkedList *list = (linkedList*)malloc(sizeof(linkedList));

It's not necessary (since C89), it clutters up the call, and it creates maintenance headaches. That can be made much simpler as follows:

linkedList *list = malloc( sizeof *list );

Fourth, note that you can write

(*list).head = NULL;
(*list).size = 0;

as

list->head = NULL;
list->size = 0;

a->b is shorthand for (*a).b, and makes things a little easier to read.

Finally,

MyLinkedListP listP = list;

is attempting to assing a value of type struct linkedList * to an object of type struct MyLinkedList *, and they aren't compatible. Based on the code you've shared, it looks like you meant to do something more like

struct MyLinkedList myList;
list.listp = list;

or

struct MyLinkedList myList = {list};

or even

MyLinkedListP listP = malloc( sizeof *listP ); // leaky abstraction from hiding
                                               // pointer behind typedef

listP->listp = list;

I suggest you take a step back and try to simplify what you're doing - you've added a bit of unnecessary complexity with your types.


  1. First of all, the struct node type isn't complete until the closing } of the struct definition, and you can't declare an instance of an incomplete type; you can, however, declare a pointer to an incomplete type. Secondly, the size to store an instance of struct node would have to be infinite, since the instance contains an instance of itself, which contains an instance of itself, which contains an instance of itself, etc.
  2. A quick rant - as a rule, you should not hide pointers behind typedefs. If the programmer has to ever be aware that they are dealing with a pointer (for example, if they need to access members of the struct type), then make that explicit in the declaration of the pointer object (look at the FILE type in stdio.h as an example - even though you only ever deal with pointers to FILEs, that pointer is not part of the FILE typedef).