Incompatible types in assignment.

Hi, i seem to be getting this error a lot, one of the main problems is i don't understand what it really means. The code i was writing below today was just a linked list to store and print out a set of phone numbers.

Code:

---


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

typedef struct L
{
        char *number ;
        struct L *next ;
} list ;

list insertnumber( char *NUMBER, list *n1 )
{
        list *n ;
        n = malloc( sizeof( list ) ) ;
        n -> number = NUMBER ;
        n -> next = n1 ;
        return n ;
        free(n) ;
}

void printl( list *n )
{
        while( n != NULL )
        {
                printf( "%s\n", n -> number ) ;
                n = n -> next ;
        }
}

int main( void )
{
        char num[15] ;
        int i ;
        list *n = NULL ;
       
        //num = malloc( sizeof( list ) ) ;

        /*while( num != '.' )
        {
                for( i = 0 ; i < 15 ; i++ )
                        num[i] = getchar() ;
                num = insertnumber( n, num ) ;
        }*/

        while( num[0] != '.' )
        {
                scanf( "%16s", num ) ;
                n = insertnumber( num, n ) ;
        }
        printl( n ) ;
        return 0 ;
}

---

I've tried it both times messing around with input. Currently it's using scanf with getchar commented out. Also should i try to be using getchar more than scanf?? I hear bad things about scanf. Right well anyway back to the point im getting these compile errors:

nlist.c: In function `insertnumber':
nlist.c:16: error: incompatible types in return
nlist.c: In function `main':
nlist.c:47: error: incompatible types in assignment

Any feedback on these errors would be much appreciated. I'm brushing up on C over Christmas and have found my real weakness is debugging, so any feedback would be welcome.


Cheers.

insertnumber returns a list struct, but you are returning a pointer to a list struct and trying to assign a list to a pointer to list.
Furthermore, look out in insertnumber. If you plan to return a pointer, you must not free it, since it would be invalid then. Moreover, you cannot or should not place any code after a return statement since it won't get executed.

You should bump up your warnings.

I also suggest you take a long hard look at what your code does. At the moment, it does not make sense.
Use a flowchart if you must.
And do not forget to free every node in the list before the program terminates.

Bump up my warnings?

Quote:

---

Originally Posted by killpoppop

Hi, i seem to be getting this error a lot, one of the main problems is i don't understand what it really means. The code i was writing below today was just a linked list to store and print out a set of phone numbers.

Code:

---


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

typedef struct L
{
        char *number ;
        struct L *next ;
} list ;

list insertnumber( char *NUMBER, list *n1 )
{
        list *n ;
        n = malloc( sizeof( list ) ) ;
        n -> number = NUMBER ;
        n -> next = n1 ;
        return n ;
        free(n) ;
}

void printl( list *n )
{
        while( n != NULL )
        {
                printf( "%s\n", n -> number ) ;
                n = n -> next ;
        }
}

int main( void )
{
        char num[15] ;
        int i ;
        list *n = NULL ;
       
        //num = malloc( sizeof( list ) ) ;

        /*while( num != '.' )
        {
                for( i = 0 ; i < 15 ; i++ )
                        num[i] = getchar() ;
                num = insertnumber( n, num ) ;
        }*/

        while( num[0] != '.' )
        {
                scanf( "%16s", num ) ;
                n = insertnumber( num, n ) ;
        }
        printl( n ) ;
        return 0 ;
}

---

I've tried it both times messing around with input. Currently it's using scanf with getchar commented out. Also should i try to be using getchar more than scanf?? I hear bad things about scanf. Right well anyway back to the point im getting these compile errors:

nlist.c: In function `insertnumber':
nlist.c:16: error: incompatible types in return
nlist.c: In function `main':
nlist.c:47: error: incompatible types in assignment

Any feedback on these errors would be much appreciated. I'm brushing up on C over Christmas and have found my real weakness is debugging, so any feedback would be welcome.


Cheers.

---

cannot free() what is in use so take that line in red out.

Quote:

---

Originally Posted by killpoppop

Bump up my warnings?

---

Every compiler typically has a setting that says how much warnings it should output. This option should always be maxed out - so that it gives as many warnings as possible.
For example, in your code, you have unreachable code, as pointed out, and the compiler should warn about that.

Have done im now returning:

return *n ;

which seems to remove the error.

in main i've now changed it to:

*n = insertnumber( num, n ) ;

again it removes the error.

These the correct steps to make?

Quote:

---

Originally Posted by Elysia

Every compiler typically has a setting that says how much warnings it should output. This option should always be maxed out - so that it gives as many warnings as possible.
For example, in your code, you have unreachable code, as pointed out, and the compiler should warn about that.

---

Ah i see that would be useful, i'll change my compiler options =] Thanks!

No, the first destroys the usefulness of a linked list.
The second will cause a runtime error when you run it.
You need to fully understand how pointers work and what they are good for.
Here are some resources:
http://cslibrary.stanford.edu/104/
http://cboard.cprogramming.com/showp...3&postcount=31

I would review them first and see if you can get a better understanding of how they work, and then see if you can understand what you do is wrong.

Oh yes, and also
http://cpwiki.sourceforge.net/Common...llocating_them

yes those are the correct changes since insertnumber() returns an object of type list instead of a pointer to it.
also remove that free(n) inside insertnumber(); shouldn't free what's in use esp. not after a return.

Haha who to believe?! =D But i am getting runtime errors. So obviously i'm wrong.

Quote:

---

Originally Posted by itCbitC

yes those are the correct changes since insertnumber() returns an object of type list instead of a pointer to it.
also remove that free(n) inside insertnumber(); shouldn't free what's in use esp. not after a return.

---

No, it is not the correct changes or the solution. It will compile, yes, but it will not run and if fixed so that it does run, it still is not correct, since it defies the purpose of the linked list.

Yup your right, thanks for the help, i have some work to do =]

Quote:

---

Originally Posted by Elysia

No, it is not the correct changes or the solution. It will compile, yes, but it will not run and if fixed so that it does run, it still is not correct, since it defies the purpose of the linked list.

---

For the sake of my learning and pure knowledge PLMK how it defies the purpose of the linked list.

I'm referring to the point that by returning a struct, a copy is made, which is not the original. Thus, if the original is actually modified, the copy is not. So you end up not with just a waste of execution time for copying the struct, not once, but possible twice, and end up with a copy of the list, as well.
This may or may not defy the purpose of a list, but it certainly might not be a good idea. Especially since a list consists of linked nodes.
If you had a pointer to it, then you would have no such problems.

Quote:

---

Originally Posted by Elysia

I'm referring to the point that by returning a struct, a copy is made, which is not the original. Thus, if the original is actually modified, the copy is not. So you end up not with just a waste of execution time for copying the struct, not once, but possible twice, and end up with a copy of the list, as well.

---

There is no copy made of the list object as it is defined inside insertnumber() only.

Quote:

---

Originally Posted by Elysia

This may or may not defy the purpose of a list, but it certainly might not be a good idea. Especially since a list consists of linked nodes.
If you had a pointer to it, then you would have no such problems.

---

The pointer to the list of nodes is contained within the structure itself so there is no deviation from the norm of linked lists.

Quote:

---

Originally Posted by itCbitC

There is no copy made of the list object as it is defined inside insertnumber() only.

---

No copy?
What do you actually think returning the pointee does?
Not to mention that (currently), returning the pointee will cause a memory leak.

Quote:

---

The pointer to the list of nodes is contained within the structure itself so there is no deviation from the norm of linked lists.

---

Yes, but if you go p->next->previous, you won't arrive at the same struct!
That just means that if you walk the struct, you will not find the same node as the copy you have made!
This can cause problems.

Quote:

---

Originally Posted by Elysia

No copy?
What do you actually think returning the pointee does?
Not to mention that (currently), returning the pointee will cause a memory leak.

---

You're correct; for some absurd reason I had it in me that list *n was malloced in main() even though IINM I said that it was defined in insertnumber() :confused:

Quote:

---

Originally Posted by Elysia

Yes, but if you go p->next->previous, you won't arrive at the same struct!
That just means that if you walk the struct, you will not find the same node as the copy you have made!
This can cause problems.

---

Now that's a doubly linked list you are talking about...there's no *prev pointer here.

Quote:

---

Originally Posted by itCbitC

Now that's a doubly linked list you are talking about... There's no *prev pointer here.

---

Yes, I know there is not, but I was merely pointing out the fact that it would not be the same node, and the easiest way to get back to a node is next, then previous. Of course, it can be walked from the beginning, as well. It was the principle I was after.

Quote:

---

Originally Posted by Elysia

Yes, I know there is not, but I was merely pointing out the fact that it would not be the same node, and the easiest way to get back to a node is next, then previous. Of course, it can be walked from the beginning, as well. It was the principle I was after.

---

Gotcha!

Quote:

---

Originally Posted by itCbitC

You're correct; for some absurd reason I had it in me that list *n was malloced in main() even though IINM I said that it was defined in insertnumber() :confused:

---

Btw, this would still cause a copy to be made, but it would avoid a runtime error.

Quote:

---

Originally Posted by Elysia

Btw, this would still cause a copy to be made, but it would avoid a runtime error.

---

Not if the malloc() happens only in main(); this way you're not dereferencing a NULL pointer and no copy is made.

No, dereferencing a NULL pointer is undefined behavior, and does not create a copy.
Returning the pointee does, however, because the function returns a struct, and thus the struct itself (the pointee) is copied, and returned to main, where it is once again copied.
If a pointer were to be returned, the pointer is copied, but the pointee is not.

Quote:

---

Originally Posted by Elysia

No, dereferencing a NULL pointer is undefined behavior, and does not create a copy.

---

I was referring not to the copy but to the runtime error when I talked about dereferencing the NULL pointer.

So are we discussing runtime errors or copies now? I do agree - mallocing will not cause a runtime error, but a copy is still made, so...
I don't know if you still have/held some belief that mallocing would solve the copy problem - which it will not, but it will, of course, solve the runtime error. In a wrong way, though, perhaps.

Quote:

---

Originally Posted by Elysia

So are we discussing runtime errors or copies now? I do agree - mallocing will not cause a runtime error, but a copy is still made, so...
I don't know if you still have/held some belief that mallocing would solve the copy problem - which it will not, but it will, of course, solve the runtime error. In a wrong way, though, perhaps.

---

I didn't mean to leave this thread abruptly, however if you see my post #21 the implication is that there's no insertnumber() function and all the malloc'ing happens ONLY in main(). Figure only is the operative keyword here. Doing it this way gets rid of the twin problem of runtime error and the copies.

Ah yes, of course. If you get rid of the function entirely.
But you want the function there, do you not? Yes, it's a good thing, so we must take care not to create copies then.

Quote:

---

Originally Posted by Elysia

Ah yes, of course. If you get rid of the function entirely.

---

Cool beans! we are on the same page then :cool:

Quote:

---

Originally Posted by Elysia

But you want the function there, do you not? Yes, it's a good thing, so we must take care not to create copies then.

---

Modularization is always good but for 3, 4 lines of code I wouldn't have gone for a separate function but again it's a matter of personal style.

Makes sense to me since it's clearer what the code does if you put it inside a function.

I've made some changes which seem to make more sense. Changes have been highlighted. Another question. About my input. I want to input numbers until the input reads in a full stop. But i'm missing something to put in my first while loop. Just kind of want it run ( e.g. while( there is input to be read in) if you get me ) only reason i have the while in there is so i can use the break statement.

So yeah here is my code.

Code:

---


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

typedef struct L
{
        char number[15] ;
        struct L *next ;
} list ;

list *insertnumber( char NUMBER, list *n1 )
{
        list *n ;
        n = malloc( sizeof( list ) ) ;
        n -> number[15] = NUMBER ;
        n -> next = n1 ;
        return n ;
}

void printl( list *n )
{
        while( n != NULL )
        {
                printf( "%s\n", n -> number ) ;
                free( n ) ;
                n = n -> next ;
        }
}

int main( void )
{
        char *num ;
        char check ;
        int i ;
        list *q = NULL ;
       
        num = calloc( 15, sizeof( char ) ) ;
       
        while(  )
        {
                check = getchar() ;
               
                if( check != '.' )
                {       
                        num[0] = check ;
                        for( i = 1 ; i < 15 ; i++ )
                                num[i] = getchar() ;
                }
                else
                        break ;
               
                q = insertnumber( *num, q ) ;
        }
       
        printl( q ) ;
       
        return 0 ;
}

---

The print function and stuff still isn't done, just wanted to make sure the pointers are now ok?

IMO you don't need the while loop. There are 2 terminal conditions: a full stop and num array bounds. Modify the for loop for these 2 cases as in

Code:

---

for(i = 0 ; i < 15 && (num[i] = getchar()) != '.'; i++)

---

If i did:

Code:

---

while( )
{
        for( i = 0 ; i < 15 && ( num[i] = getchar() ) != '.' ; i++ )
                num[i] = getchar() ;
        q = insertnumber( *num, q ) ;
}

---

surely there needs to be a loop there otherwise it will only take on phone number then stop.

alright so if I understand you correctly here's an updated version using the while loop.
here loop breaks on end of file; else store input in num until a full stop or out of bounds.

Code:

---

int check, i = 0;
while ((check = getchar()) != EOF)
{
    if (check != '.' && i < 15)
        num[i++] = check;
    else
        q = insertnumber(*num, q) ;
}

---

question is what you would do if input has 10 chars only, would you still create a linked list node or break out of the program?

I'll check your loop out in a sec. Currently im using.

Code:

---


while( num[0] != '.' )
        {
                scanf( "%15s", num ) ;
                if( num[0] != '.' )
                        q = insertnumber( *num, q ) ;
                else
                        break ;
        }

---

Would that work?

Depends what you're looking for in your code, which will exit out only if the first char num[0] is a period but not if the period is in the middle like num[10].

As far as i can see with your loop it wont terminate at a full stop.

Quote:

---

Originally Posted by killpoppop

As far as i can see with your loop it wont terminate at a full stop.

---

Correct it will terminate only on EOF and IINM a full stop is the terminal condition for node creation ie when you see a period you invoke insertnumber()??

From what I have seen, I recommend something like this right now.

Code:

---

int maybe;
unsigned area, prefix, exchange;
char num[15];

maybe = scanf("(%3u)%3u-%4u", &area, &prefix, &exchange);
while (maybe == 3) {
  sprintf(num, "(%u)%u-%u", area, prefix, exchange);
  insertnumber(num, q);
  maybe = scanf("(%3u)%3u-%4u", &area, &prefix, &exchange);
}

if (scanf("%c", &maybe) == 1 && maybe == '.') {
  /**success, else do failure operations. **/
}

---

It might not be perfect, but you need a coordinated approach to input right now. Given that a full stop is the sentinel value for a loop, we need to do a priming read. If the priming read is successful, then we enter the loop, and only exit the loop when a read fails.

When the read does fail, we have to read the next character to determine if the reads were successful. For consistency this is done with scanf. I believe the rest is self explanatory. It should be easy enough to code the logical inverse if you only need to do something when the loop fails to read all the input.