passing variables through functions...

Discussion in 'C Programming' started by Yodai, Jan 15, 2004.

  1. Yodai

    Yodai Guest

    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;
    }
    Yodai, Jan 15, 2004
    #1
    1. Advertising

  2. Yodai

    void Guest

    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);
    >
    >
    > }


    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
    --
    lint's little mind is blown.
    void, Jan 15, 2004
    #2
    1. Advertising

  3. Yodai

    Yodai Guest

    [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.


    /*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
    Yodai, Jan 15, 2004
    #3
  4. Yodai

    void Guest

    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.

    >
    >
    > /*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? */
    >

    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);

    }
    void, Jan 15, 2004
    #4
  5. Yodai wrote:
    > 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"},
    > };
    >


    <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>


    > for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)


    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)

    {
    return cadena.string;

    Here you are returning a pointer to a local variable.

    }
    }
    > return (escape);


    And here too.

    >
    > }
    >


    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
    Bruno Desthuilliers, Jan 15, 2004
    #5
  6. [OT] passing variables through functions...

    Bruno Desthuilliers <> scribbled the following:
    > <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>


    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".

    --
    /-- Joona Palaste () ------------- Finland --------\
    \-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
    "The truth is out there, man! Way out there!"
    - Professor Ashfield
    Joona I Palaste, Jan 15, 2004
    #6
  7. Yodai

    Yodai Guest

    //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.....
    Yodai, Jan 15, 2004
    #7
  8. Yodai

    Grumble Guest

    Bruno Desthuilliers wrote:

    > Yodai wrote:
    >
    >> for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)

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


    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!
    Grumble, Jan 15, 2004
    #8
  9. Yodai

    osmium Guest

    Yodai writes:

    > 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);

    <snip>

    You are returning an auto variable. Make it static or ......
    osmium, Jan 15, 2004
    #9
  10. Yodai

    CBFalconer Guest

    void wrote:
    > 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);
    > > }

    >
    > 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.


    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.

    --
    Chuck F () ()
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net> USE worldnet address!
    CBFalconer, Jan 15, 2004
    #10
  11. Re: [OT] passing variables through functions...

    Joona I Palaste wrote:
    > Bruno Desthuilliers <> scribbled the following:
    >
    >><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>

    >
    >
    > You mean "POV", not "POW".


    You're absolutely right !-)
    Bruno Desthuilliers, Jan 15, 2004
    #11
  12. Grumble wrote:
    > Bruno Desthuilliers wrote:
    >
    >> Yodai wrote:
    >>
    >>> for(i = 0; i < sizeof(cadena) / sizeof(cadena[0]); i++)

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

    >
    >
    > 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!
    >


    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.
    Bruno Desthuilliers, Jan 15, 2004
    #12
  13. void wrote:
    > 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);
    >>
    >>
    >> }

    >
    >
    > You are returning pointer which points on stack -


    <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.


    <pedantic>
    the problem has nothing to do with 'stack', but with the fact that this
    code returns a pointer to an automatic local variable.
    </pedantic>

    > Make variables cadena and escape global or static, and then it should
    > work correctly.


    Yuck !

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


    Bruno
    Bruno Desthuilliers, Jan 15, 2004
    #13
  14. Yodai

    j Guest

    "Yodai" <> wrote in message
    news:BVwNb.2638189$...
    > //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.....
    >
    >


    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''.
    j, Jan 15, 2004
    #14
    1. Advertising

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

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. ILLOGIC
    Replies:
    1
    Views:
    355
    Rob Williscroft
    Jun 1, 2004
  2. James
    Replies:
    7
    Views:
    319
    James
    Nov 5, 2006
  3. Henry

    Passing arrays through functions

    Henry, Sep 15, 2003, in forum: ASP General
    Replies:
    3
    Views:
    152
    Ray at
    Sep 15, 2003
  4. A Web Master
    Replies:
    4
    Views:
    223
    A. Webmaster
    Jan 28, 2004
  5. kasper48
    Replies:
    2
    Views:
    106
    Moses
    Oct 21, 2006
Loading...

Share This Page