What's the problem with the code??

Go To Last Post
15 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi everyone!

I'm making my own library with the functions that I use the most.
One of this function is as checksum routine.

unsigned char* checksum (unsigned char data[], short length){

unsigned short i=0;
unsigned char aux;
unsigned char chk[2];

for(i=0; i

when I compile the code I've got 2 warnings:
-function returns addres of local variable
-return from incompatible pointer type

The first one I understand it, but how to fix it?
Do i have to declare a global unsigned char chk[], at the main.c??

Thanks in advice

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It's just two bytes, why don't you return it as uint16_t (unsigned short)?

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
function returns addres of local variable
Because you create "chk" on the stack and return that address. When the function returns, that portion of the stack is no longer valid.
(Someone correct me if i'm wrong here)

I also don't see why you need to return a pointer, just return the value.

I don't get the second warning.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Also your checksums are random since you don't initialize aux to any known value.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Thanks to all for the replies
- wek:
I already do this, and it works fine, but I wanted to understand why it didn't work the way it is.

-thygate:
Then it's a problem that i use the stack...And what's gonna happen if I declare it static??

I tried declaring static unsigned char chk[2], and I don't get anymore the first warning. But the second persists.

-Jepael:
You're right. I have to.

Thanks to all!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
I tried declaring static unsigned char chk[2], and I don't get anymore the first warning.
Because the variable is now persistent, and is no longer allocated on the stack in the function. So the address will stay valid when the function returns.

One usually passes a pointer to allocated memory, so the function can edit the value. If you need to return newly allocated memory, you would use the heap (malloc).
But then you'll have to free the memory somewhere else.

"Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

Last Edited: Fri. Jan 14, 2011 - 04:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

felipedelaxe wrote:
I tried declaring static unsigned char chk[2], and I don't get anymore the first warning. But the second persists.
What happens if you add static to the return type in your function declaration?

Smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

thygate:
- Thanks for the explanation, now I understand this a bit better.

Smileymicros:
- If I type return static chk (Is that what you mean??) I get an error: expected expression before 'static'. What does it mean??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

unsigned static char* checksum (unsigned char data[], short length)

Thats what I meant, but since I would NOT do this they way you are doing it, I don't know if it will work or not. I would just calculate a uint16_t and return that.

Smiley

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:
What happens if you add static to the return type in your function declaration?
You can't do that. It is a type, not a variable (which is I think also why you get the warning). The compiler would think that you want the function static.

When you send a char array into a function that takes a char *, a local variable is created and the value of that variable is set to the address of the first element of the array. When you return an array from a function that returns a char *, there is no real variable so the compiler can't set its value. In the end it would probably work (as long as the array is static) since the correct value would be returned in the correct registers so the assignment to the real variable should happen.

Regards,
Steve A.

The Board helps those that help themselves.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

felipedelaxe wrote:
Then it's a problem that i use the stack...And what's gonna happen if I declare it static??

I tried declaring static unsigned char chk[2], and I don't get anymore the first warning.

As others noted, a non-static variables' lifespan ends when the function returns, so the pointer returned to the calling function may a few moments later point to a memory already used for other purpose (the "return value" overwritten). Static local variables are in fact global variables invisible to other functions so they are "alive" "forever". Formally, that would be the solution to your problem, and it works as expected.

However, it is not a good style generally (there might be exceptions). The point is, that the calling function should be responsible for allocating space for return values, or at least it should be capable of de-allocating that space. I know that this does not make much sense at first, but wait until you start to write reusable functions (libraries), then you will start to value such recommendations.

felipedelaxe wrote:

But the second persists.
You need to copy/paste exactly what do you try to compile.

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I've got it!
Now it's compiled without warnings or errors. The second warning was because in the header file of the library the declaration of the function wasn't exactly the same.

But, even it's running, since a lot of you said that it's not the better way of programming I'm gonna do it returning an unsigned int.

But I still have a question: If I want a function wich returns a char array, how can I do this??

Thanks to everyone. I'm learning a lot!!

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Quote:

If I want a function wich returns a char array, how can I do this??

Since automatic allocation on the stack (what most would call "local variables") is out of the question, you have two options:

- A global array, and you return the address of it.
- A dynamically allocated array (e.g. with malloc()), and you return the address of it.

Both variants have their advantages/disadvantages.

Often, this is handled not by returning the arra (as a return value of a function), but instead making the caller responsible for allocating the memory needed and then pass a pointer to this memory as a parameter. E.g. as in a lot of standard library functions like memcpy, sprintf etc... The drawback here is that the size of the allocated memory is determined before the call.

As of January 15, 2018, Site fix-up work has begun! Now do your part and report any bugs or deficiencies here

No guarantees, but if we don't report problems they won't get much of  a chance to be fixed! Details/discussions at link given just above.

 

"Some questions have no answers."[C Baird] "There comes a point where the spoon-feeding has to stop and the independent thinking has to start." [C Lawson] "There are always ways to disagree, without being disagreeable."[E Weddington] "Words represent concepts. Use the wrong words, communicate the wrong concept." [J Morin] "Persistence only goes so far if you set yourself up for failure." [Kartman]

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

JohanEkdahl wrote:
Often, this is handled not by returning the arra (as a return value of a function), but instead making the caller responsible for allocating the memory needed and then pass a pointer to this memory as a parameter. E.g. as in a lot of standard library functions like memcpy, sprintf etc... The drawback here is that the size of the allocated memory is determined before the call.
I would not call it a drawback. Often (in mcus, almost always) there is a good reason to limit the memory usage. In the scheme discussed here, this converts to array allocated by the caller and its size passed to callee, see snprinf() and strncpy()/strlcpy() (which are oh so typically refused by the "true C" "programmers"...)

JW

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yep I agree. That would be the best way to do it other than returning an int value. Something like :

unsigned char* checksum (unsigned char data[], short length, unsigned char out_chk[])

Then you would call it like :

unsigned char chk[2];
checksum(data, length, chk);