passing variables through functions...

Y

Yodai

Hi all....

I got this function I worked on that has to receive a variable and return a
value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly positive
that the error is in Detect_type(). What I do is:



char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};


for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);


}


//and then from somewhere in my main I call this function to translate
values for me like this:

case 'a' :
{
volatile char tipus = *R1TIPUS ;
sprintf(NewKey, "%u", Detect_type(tipus));
memcpy(Key, NewKey, 10);
break;
}
case 'b' :
{
volatile char tipus = *R2TIPUS ;
sprintf(NewKey, "%u", Detect_type(tipus));
memcpy(Key, NewKey, 10);
break;
}
 
V

void

U¿ytkownik Yodai napisa³, On 2004-01-15 13:08:
Hi all....

I got this function I worked on that has to receive a variable and return a
value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly positive
that the error is in Detect_type(). What I do is:



char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};


for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);


}
Click to expand...


You are returning pointer which points on stack - this is the problem.
Make variables cadena and escape global or static, and then it should
work correctly.

Best regards
Darek Ostolski
 
Y

Yodai

[snip]
You are returning pointer which points on stack - this is the problem.
Make variables cadena and escape global or static, and then it should
work correctly.
Click to expand...

/*hum... Ok.. I understand how to make escape a global variable. Just have
to declare it in my main() as*/


char escape [6] ;

/* but how do I declare cadena globaly? */



Cheers!

Yodai
 
V

void

U¿ytkownik Yodai napisa³, On 2004-01-15 13:42:
[snip]
You are returning pointer which points on stack - this is the problem.
Make variables cadena and escape global or static, and then it should
work correctly.
Click to expand...


/*hum... Ok.. I understand how to make escape a global variable. Just have
to declare it in my main() as*/


char escape [6] ;

/* but how do I declare cadena globaly? */
Click to expand...
make it static. example:
char* Detect_type(volatile char tipus)
{
int i;
static char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
};
static struct decide cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};


for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);

}
 
B

Bruno Desthuilliers

Yodai said:
Hi all....

I got this function I worked on that has to receive a variable and return a
value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly positive
that the error is in Detect_type(). What I do is:



char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};
Click to expand...

<OT>
From a design POW, and depending on project-specific constraints, you
may want to have this in a conf file so you don't have to rebuild if
something changes...
for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
Click to expand...

You may also want to pre-compute this :
size_t nb_items = sizeof(cadena) / sizeof(cadena[0]);
for(i = 0; i < nb_items; i++)

{
if(cadena.num == tipus)
Click to expand...

{
return cadena.string;

Here you are returning a pointer to a local variable.

}
}
return (escape);
Click to expand...

And here too.
}
Click to expand...

Variables that are local to the callee are destroyed as soon as control
returns to the caller. Well, not exactly... they may be overwritten any
time after control has returned to the caller.

Anyway, you don't want to return a pointer to a block of memory that is
no more under your control, and that may be used for anything else.

The two solutions are :
1/ pass the function a pointer to a block of memory that you control and
that is big enough to hold the data.
2/ let the function create this block and destroy it yourself when you
don't need it anymore.

/* Exemple_1.c */

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

char* fun1(char *retval)
{
char *data = "data";
assert(retval != NULL && strlen(retval) >= strlen(data));
strcpy(retval, data);

/* this may come in handy for uses like
* printf("%s\n", fun1(...));
*/
return retval;
}

int main(void)
{
char val[20];
printf("%s\n", fun1(val));
return 0;
}

/* Exemple_2.c */

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

char* fun2(void)
{
char *data = "data";
char *retval = malloc(strlen(data) + 1);
if (retval != NULL) {
strcpy(retval, data);
}
return retval;
}

int main(void)
{
/* note that you must keep trace of
* the pointer returned by fun2(),
* so you can free the memory it points to
*/
char* val = fun2();
if (val != NULL) {
printf("%s\n", val);
free(val); /* free the memory */
val == NULL /* not necessary here but a good habit */
}
else {
printf("malloc failed in fun2()");
}
return 0;
}


HTH
Bruno
 
J

Joona I Palaste

Bruno Desthuilliers said:
<OT>
From a design POW, and depending on project-specific constraints, you
may want to have this in a conf file so you don't have to rebuild if
something changes...
</OT>
Click to expand...

You mean "POV", not "POW". A POW is a Prisoner Of War. I understand
the difference between V and W might be difficult for non-English-types
such as you and me. It's particularly tricky in words like "view", with
both letters. When I was back in school I kept trying to spell it
"wiev".
 
Y

Yodai

//Ok.... Now they're static. Well, I've avoid using a varibale for the
return, so I only have to make static

char* Detect_type(volatile char tipus)
{
int i;
static char escape[10] = "empty";
static struct decide {
char num;
char *string;
} cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};



for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);

}


// And try to pick it up this way:


switch (*(Key + 3))
{

case 'a' :
{
volatile char tipus = *R1TIPUS ;
if (Get_tipus1() == 0xFF)
{
sprintf(NewKey, "%s", "empty"); //this is for
double security, just to make sure nothing unwanted comes through
memcpy(Key, NewKey, 6);
break;
}
else
{
sprintf(NewKey, "%u", Detect_type(tipus));
memcpy(Key, NewKey, 10);
break;
}
}
}


//Still, no success.... Though it compiles and all, it prints bogus
numbers.....
 
G

Grumble

Bruno said:
Yodai said:
for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
Click to expand...

You may also want to pre-compute this :
size_t nb_items = sizeof(cadena) / sizeof(cadena[0]);
for(i = 0; i < nb_items; i++)
Click to expand...

Are you afraid that your compiler might produce code to perform the
division several times at run time?

Since both sizeof(cadena) and sizeof(cadena[0]) are known at compile
time, I would be very disappointed if my compiler did not perform the
division at compile time!
 
O

osmium

Yodai said:
I got this function I worked on that has to receive a variable and return a
value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly positive
that the error is in Detect_type(). What I do is:



char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};


for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);
Click to expand...

<snip>

You are returning an auto variable. Make it static or ......
 
C

CBFalconer

void said:
U¿ytkownik Yodai napisa³, On 2004-01-15 13:08:
I got this function I worked on that has to receive a variable
and return a value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly
positive that the error is in Detect_type(). What I do is:

char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};

for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);
}
Click to expand...


You are returning pointer which points on stack - this is the
problem. Make variables cadena and escape global or static, and
then it should work correctly.
Click to expand...


Not quite. The cadena.string terms point to statically stored
strings, and are ok. The escape[] array does not. It should be
declared as:

char *escape = "empty";

Making those items static is probably the best solution anyhow.
No need to generate all that initialization code and execute it on
each invocation.

The volatile in the function header makes no sense whatsoever.
unsigned would.
 
B

Bruno Desthuilliers

Grumble said:
Bruno said:
Yodai said:
for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
Click to expand...


You may also want to pre-compute this :
size_t nb_items = sizeof(cadena) / sizeof(cadena[0]);
for(i = 0; i < nb_items; i++)
Click to expand...


Are you afraid that your compiler might produce code to perform the
division several times at run time?

Since both sizeof(cadena) and sizeof(cadena[0]) are known at compile
time, I would be very disappointed if my compiler did not perform the
division at compile time!
Click to expand...

Of course, sizeof being an operator :(

I usually use this idiom to avoid repeated function calls... But of
course it doesn't mean anything here.
 
B

Bruno Desthuilliers

void said:
U¿ytkownik Yodai napisa³, On 2004-01-15 13:08:
Hi all....

I got this function I worked on that has to receive a variable and
return a
value, but it doesn't seem to work.
It does compile, but it doesn't return a thing..... I am fairly positive
that the error is in Detect_type(). What I do is:



char* Detect_type(volatile char tipus)
{
int i;
char escape[6] = "empty";

struct decide {
unsigned num;
char *string;
}
cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};


for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);


}
Click to expand...



You are returning pointer which points on stack -
Click to expand...


<pedantic>
The C language has no notion of 'stack'. As a C programmer (I mean
sticking with ISO C), you don't know anything about things like 'stack'
and 'heap'.
</pedantic>

(still this is usually how it works on most microcomputers)
this is the problem.
Click to expand...

<pedantic>
the problem has nothing to do with 'stack', but with the fact that this
code returns a pointer to an automatic local variable.
Make variables cadena and escape global or static, and then it should
work correctly.
Click to expand...

Yuck !

<op>
*Dont* do that - unless you have *no* other choice.
</op>


Bruno
 
J

j

Yodai said:
//Ok.... Now they're static. Well, I've avoid using a varibale for the
return, so I only have to make static

char* Detect_type(volatile char tipus)
{
int i;
static char escape[10] = "empty";
static struct decide {
char num;
char *string;
} cadena [] = {
{0x00 , "stv1pk"},
{0x02, "stv1rms"},
{0x04, "itv1rms"},
{0x06, "ian1pk"},
{0x08, "ian1rms"},
{0x0A, "i1pk"},
{0x0C, "i1rms"},
{0x0E, "vccoff"},
{0x10, "vccon"},
{0x12, "rearme"},
{0x14, "magneto"},
{0x16, "offmanual"},
{0x18, "interno"},
{0x1A, "remotein"},
};



for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)
if(cadena.num == tipus) return cadena.string;

return (escape);

}


// And try to pick it up this way:


switch (*(Key + 3))
{

case 'a' :
{
volatile char tipus = *R1TIPUS ;
if (Get_tipus1() == 0xFF)
{
sprintf(NewKey, "%s", "empty"); //this is for
double security, just to make sure nothing unwanted comes through
memcpy(Key, NewKey, 6);
break;
}
else
{
sprintf(NewKey, "%u", Detect_type(tipus));
memcpy(Key, NewKey, 10);
break;
}
}
}


//Still, no success.... Though it compiles and all, it prints bogus
numbers.....
Click to expand...


Detect_type returns a pointer to char, and yet in the sprintf call in
your switch statement, you are using the ``%u'' conversion specifier.

``sprintf(NewKey, "%u", Detect_type(tipus));''

This is a no-no because it invokes undefined behaviour. You want ``%s''.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Looping problem (revisited for better reading) 0
a looping problem..... 0
loop variable 5
encryption keysize restriction 3
Test yourself for Performance 0
Finding Duplicate Messages off of COM port 7
comp.lang.c Answers (Abridged) to Frequently Asked Questions (FAQ) 0
comp.lang.c Answers (Abridged) to Frequently Asked Questions (FAQ) 0

Members online

No members online now.
Total: 42 (members: 0, guests: 42)
Robots: 175

Forum statistics

Threads
473,754
Messages
2,569,527
Members
45,000
Latest member
MurrayKeync

Latest Threads

Top