passing pointers... where did my data go? 2

[sarahnade]

I'm trying to print out the contents of a linked list.To do that, I have created pointers of the lists in the main function here:

int main(void){

List *l1, *l2;

l1 = (List *)malloc(sizeof(node));
l2 = (List *)malloc(sizeof(node));

l1->head = (node *)malloc(sizeof(node));
l2->head = (node *)malloc(sizeof(node));

l1->head->str = (char *)malloc(80 * sizeof(char));
l2->head->str = (char *)malloc(80 * sizeof(char));

//stuff...

return 0;

}

and passed them into the add function here:

void addEnd(List *strList, char *string){
        node *current = strList->head;
        while(1){
                if(current == NULL) break;           
                else current = current->next;        
        }
        current = (node *)malloc(sizeof(node));
        current->str = (char *)malloc(80 * sizeof(char));
        current->str = string;
}

when I try to print it in the main

printf("head: %s", l1->head->str);

it prints "head:" Where is my data getting lost? I haven't quite "grasped" pointers yet, so I'm sure this is a silly question. Thanks for your help!

 

[Salem]

1. Don't cast malloc in C. The only thing it does it to potentially hide your failure to include stdlib.h

2. Consider this

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

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

typedef struct List {
  node *head;
}List;

/* alloc a new node and space for the string, and copy it */
node *makenode ( char *string ) {
  node *result = malloc ( sizeof *result );
  result->next = NULL;
  result->str  = malloc ( strlen(string) + 1 );
  strcpy ( result->str, string );
  return result;
}

void add ( List *list, char *string ) {
  node *tail = list->head;
  node *newnode = makenode ( string );
  if ( tail == NULL ) {
    /* list empty, new node is head */
    list->head = newnode;
  } else {
    /* find the tail and append */
    while ( tail->next != NULL ) tail = tail->next;
    tail->next = newnode;
  }
}

void print ( List list ) {
  node *curr = list.head;
  while ( curr ) {
    printf( "%s\n", curr->str );
    curr = curr->next;
  }
}

int main ( ) {
  List l1 = { NULL };
  add ( &l1, "hello" );
  add ( &l1, "world" );
  print ( l1 );
  return 0;
}

--

 

[sarahnade]

Works! Thanks again.

 

[WebDrake]

Don't cast malloc in C.

Can you explain a bit further? It was my understanding that since malloc returns a type of (void *), it was always necessary to cast it.

How can it hide a failure to include stdlib.h? Surely if you don't include that header, the compiler will return an error message that malloc has been implicitly declared, or similar?

 

[Salem]

> It was my understanding that since malloc returns a type of (void *), it was always necessary to cast it.
C (C++ is different) provides automatic casting of void* to/from type*
Therefore there is no need to do the cast yourself.

One frequent cause of difficulty is people compiling their C code with a C++ compiler, and generally not realising it.

> How can it hide a failure to include stdlib.h?
If you don't include stdlib.h, the compiler generates the implicit function declaration (not a prototype) of
[tt]int malloc();[/tt]

If you DONT cast malloc, you get a 'can't convert int to pointer' warning. This is your hint to prototype it properly by including the correct header.
If you DO cast, you get nothing - the cast has hidden the problem.

On machines where pointers and integers are different sizes, the code is now seriously broken. It's also broken on machines which use different registers for returning pointers and integers.

--

 

[WebDrake]

OK, but there is a tradeoff here, right?---because if we don't cast type, the code is automatically not usable in C++.

Anyway, doesn't the implicit declaration of malloc generate its own warning message? I guess it depends on code size whether this is easy to keep track of, but I don't see it's that much of a problem.

 

[Salem]

> because if we don't cast type, the code is automatically not usable in C++
Ah yes, the random compatibility with C++ argument. Attempting to write polyglot code usually ends up in something which is neither good C nor good C++.

If complete compatibility with C++ could be achieved with a simple cast, then all would be well. But there are many valid C programs which either don't compile at all in C++, or which produce different results when compiled as C++.

Besides which, programmers need to realise that in C++, constructors are only called by new, and not malloc - yet more bugs waiting for the unwary.

> Anyway, doesn't the implicit declaration of malloc generate its own warning message?
It should do with C99 compilers, but it will be a while yet before they're in common usage.


Another (poor) argument in favour of casting is that it helps you to track down malloc calls to fix the sizeof. For example:

short int *p;
/* some code */
p = (short int*)malloc( 10 * sizeof(short int) );

The argument goes that if the type of p changes to say int, then you'd get a warning if you forgot to change the malloc call and the cast. The aim being to end up with this.

int *p;
/* some code */
p = (int*)malloc( 10 * sizeof(int) );

However, if the programmer is momentarily distracted, in a hurry, or simply not paying attention, then this sorry mess can happen.

int *p;
/* some code */
p = (int*)malloc( 10 * sizeof(short int) );

The problem is, the type is mentioned in 3 different places, but only two of them have changed. For sure it now compiles without warning, but the code has been fatally broken. What is more, finding the problem will be harder because the programmer will be convinced that there won't be a problem since he just changed that line!.

Mentioning the type in three different places is just excess verbosity, and a maintenance nightmare waiting to happen.

Contrast with my previous example, the type is mentioned only once.
> [tt]node *result = malloc ( sizeof *result );[/tt]
The general form of this being
[tt]p = malloc ( numRequired * sizeof *p );[/tt]
The whole argument disappears since in the actual malloc call itself, there is no mention of the type, so there is nothing which actually needs changing.

--

 

[WebDrake]

Attempting to write polyglot code usually ends up in something which is neither good C nor good C++.

I wasn't really thinking about "polyglot code"; I was thinking more along the lines of code like the GNU Scientific Library (GSL), which is written in ANSI C, but which can also be effectively used with C++.

I accept that there will always be (sometimes quite subtle) compatibility issues, but it seems to me that it's wrong-headed to deliberately write code which---e.g. because of the casting issue---is definitely going to be incompatible between the two languages.

A case in point I remember is that in C one can have a function pointer of the form,

int (*f)(void *);

And then write,

f = &g;

where g() was declared as,

int g(int *);

In C this is fine as the void * can be used as a "general" pointer where you don't know the type. However, in C++ this returns an error because of the type differences in the variables accepted as input by f and g.

I remember that GSL gets round this by a method like this: they would declare,

int g(void *);

but then the first line of g would be something like,

int g(void *a) {
    int *b = (int *) a;

    /* rest of function uses b */
}

Besides which, programmers need to realise that in C++, constructors are only called by new, and not malloc - yet more bugs waiting for the unwary.

Surely that's simply a matter of learning the difference between objects and the simpler types that carry across from C like int, double, pointers, arrays, etc.---a lesson which needs to be learned anyway.

I accept your point about the possibility to build in extra errors through forgetting to change one of the type declarations in malloc---I've made that mistake myself---but I think it's worth the risk given that, when finished, I will often have a library which can be called from a C++ program as effectively as from C.

 

[DaveSinkula]

In C this is fine as the void * can be used as a "general" pointer where you don't know the type.

In C, a good compiler -- or one set to high error/warning level -- or linter will not like this either.

main.c:8: warning: assignment from incompatible pointer type

main.c 8 error [Warning 546] Suspicious use of &
main.c 8 error [Error 64] Type mismatch (assignment) (int (*)(void *) = int (*)(int *))

Warning W8001 main.c 8: Superfluous & with function in function main
Warning W8075 main.c 8: Suspicious pointer conversion in function main

 

[Salem]

> which is written in ANSI C, but which can also be effectively used with C++.
There's a big difference between having a bunch of C code which you happen to compile with a C++ compiler (hence it is actually c++ code), than adopting a true mixed language approach.
If you really want to mix C and C++, then you should start here

> I will often have a library which can be called from a C++ program as effectively as from C.
You don't need to mung your code just to make it usable from C++. With the use of extern "C", you'll be able to call your C functions from your C++ code (and from your C code), and still compile it with your C compiler.

Unfortunately, that FAQ glosses over the problems of using C++ as "a better C". Here is a near complete catalogue of issues you need to cope with.

> int *b = (int *) a;
Yes, and every time you do this, you weaken both languages ability to tell you that you might be doing something wrong. Essentially, its telling the compiler "shut up, I know what I'm doing".

C++ added 4 new casts (static_cast, dynamic_cast, const_cast and reinterpret_cast) to act in very specific ways in a safe manner. The C style cast is unsafe. Indeed, pc-lint has a specific warning "1924 C-style cast -- A C-style cast was detected."

--

 

[WebDrake]

In C, a good compiler -- or one set to high error/warning level -- or linter will not like this either.

I'm sure I remember successfully compiling stuff like this with gcc -ansi -pedantic -Wall. It generates warnings, of course, as it should, but the code compiles and works fine.

There's a big difference between having a bunch of C code which you happen to compile with a C++ compiler (hence it is actually c++ code), than adopting a true mixed language approach.

Well, suppose I want the freedom to choose---maybe to compile the code with a C++ compiler, maybe to link it in the manner you suggest ... What then?

 

[Salem]

> maybe to compile the code with a C++ compiler, maybe to link it in the manner you suggest ... What then?
Personally, I'd go for a proper mixed language approach, compile each file using the appropriate compiler and heed all the warnings. Warning counts >0 are basically unacceptable to me.

But if you want to cast everything down to the lowest common denominator just to simplify the build process to a single compiler, and ignore what warnings remain, then that's your choice.

> It generates warnings, of course, as it should, but the code compiles and works fine.
The "works for me today with my current compiler" argument. Change anything and you're up to your ass in alligators once more. Not to mention, getting to ask later on "why doesn't this work with the foo compiler?"

The problem with a sea of warnings is spotting the new one which you should really be paying attention to. It's easy for me to spot new warnings since I don't have in stable code.

> Well, suppose I want the freedom to choose
I would consider that you're now making a more informed choice. I'm not going to tell you which choice to make, but I think I've pointed out an alternative you might consider.

--

 

[WebDrake]

I would consider that you're now making a more informed choice. I'm not going to tell you which choice to make, but I think I've pointed out an alternative you might consider.

Yes, and I'm very grateful for your advice---thank you very much! I'm not questioning because I necessarily want to disagree, I'm questioning because I want to understand your arguments properly and what the different points of view are.

 

[cpjust]

I'm in the same mindset as Salem. I always compile with the highest warning level and treat warnings as errors. Only builds with 0 errors and 0 warnings are acceptable to me.

I'd write whatever I can with C++, and only what I need to with C. You can always optimize later if you need to.