what is wrong with the following very short program

Discussion in 'C Programming' started by wkevin, Feb 10, 2014.

  1. wkevin

    wkevin Guest

    Hi All,

    Any ideas what is wrong with the following very short program ?

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

    typedef struct my_struct myStruct;
    struct my_struct {
    int val;
    };



    myStruct *f2(void)
    {
    myStruct *dst = (myStruct*)calloc(5, sizeof(myStruct));
    dst->val = 5;
    return dst;
    }


    void f1(myStruct *dst)
    {
    dst = f2();
    printf("dst->val=%d\n", dst->val);
    }


    int main()
    {
    myStruct *src = (myStruct*)malloc(sizeof(myStruct));
    myStruct *dst;
    f1(dst);
    printf("dst->val in main=%d\n", dst->val);

    }


    Any ideas why, when running this program, I get:

    dst->val=5
    dst->val in main=1

    Why is it 1 in main and not 5 ?
    I would expect that in main it will also be 5.
    What should I do so that in main it will be 5 ?

    regards,
    Kevin
     
    wkevin, Feb 10, 2014
    #1
    1. Advertisements

  2. wkevin

    Eric Sosman Guest

    This is a slightly obscured version of Question 4.8 on the
    comp.lang.c Frequently Asked Questions (FAQ) page at

    <http://www.c-faq.com/>
     
    Eric Sosman, Feb 10, 2014
    #2
    1. Advertisements

  3. wkevin

    wkevin Guest

    Hi,
    Thanks for the quick response.

    Still, after fixing according to 4.8, the following does not work .Any ideas what should be done to fix it ?

    typedef struct my_struct myStruct;
    struct my_struct {
    int val;
    };



    myStruct *f2(void)
    {
    static myStruct dst;
    dst.val = 6;
    return &dst;
    }


    void f1(myStruct *dst)
    {
    dst = f2();
    printf("dst->val=%d\n", dst->val);
    }


    int main()
    {
    myStruct dst;
    f1(&dst);
    printf("**dst.val in main=%d\n", dst.val);

    }
     
    wkevin, Feb 10, 2014
    #3
  4. wkevin

    Jorgen Grahn Guest

    [...]

    When I compile it with the appropriate options set, gcc says:

    foo.c: In function 'main':
    foo.c:29:19: warning: unused variable 'src'
    foo.c:31:11: warning: 'dst' is used uninitialized in this function

    Let the compiler do its job! You have better things to do.

    /Jorgen
     
    Jorgen Grahn, Feb 10, 2014
    #4
  5. wkevin

    Eric Sosman Guest

    The changes you made are not "according to 4.8" at all, which
    leads me to suspect you didn't understand 4.8's point. Go back and
    read it again, giving more attention to the answer's first sentence
    than to the stuff that follows.
     
    Eric Sosman, Feb 10, 2014
    #5
  6. wkevin

    wkevin Guest

    Sorry, I read it. I am a newbie to C. I don't know what should be
    done to fix the program. I will appreciate of anyone can fix this
    very short program so that it will work

    Regards,
    Kevin
     
    wkevin, Feb 10, 2014
    #6
  7. wkevin

    BartC Guest

    Change f1() to:

    myStruct *f1(void) {
    myStruct *dst;
    dst = f2();
    printf("dst->val=%d\n", dst->val);
    return dst;
    }
    Change the above to:

    dst=f1();

    The problem is that dst in main() is not being set up to point to anything.
    (You got the right idea with f2(), but interposing f1() between main() and
    f2() wasn't done properly.)
    I just got a crash because dst is not set up.
     
    BartC, Feb 10, 2014
    #7
  8. wkevin

    James Kuyper Guest

    calloc() can fail. If it does, it returns a null pointer. If dst is
    null, then the following statement has undefined behavior. Always check
    for the possibility of failure whenever calling malloc(), calloc(), or
    realloc(), and make sure NOT to dereference the pointers they return if
    they are null.
    The variable dst in this program contains a copy of the pointer value
    passed to it by the calling routine. It then replaces that pointer with
    the one returned by f2(). As a result, there's no point in having the
    caller pass that value. You might as well define dst inside of f1,
    rather than making it a function argument. That's not the correct way to
    fix this problem, but it's a key symptom of what's wrong with this function.

    When f1 returns, the variable named "dst" declared in this function
    ceases to exist, and the information stored in that value is lost.
    Therefore, it's no longer possible to free() the memory allocated by
    f2(). This is a memory leak. That's less important than the other
    problem, but s
    At this point, dst is uninitialized; that means that it's value is
    indeterminate.
    Since the value of dst is indeterminate, it might be a trap
    representation, a value that does not represent any valid memory
    location. Attempting to read a trap representation has undefined
    behavior. There are real world machines where that could cause your
    program to be aborted.

    That statement causes a the value of dst in main() to be copied to the
    variable named dst in f1(). There is otherwise no connection between
    those two variables, they are two entirely different variables that
    happen to have the same name.

    Therefore, the fact that f1() changes the value of f1()'s variable named
    "dst" has no affect whatsoever on the value of main()'s variable named
    dst. That variable remains is uninitialized.
    Even if dst doesn't contain a trap representation, it points at some
    unknown location on your machine. That location is almost certainly NOT
    the same as the location allocated by the call to calloc() in f2(). It
    might not be a piece of memory that your program has no right to access;
    attempting to access it could get your program aborted. The behavior of
    attempting to dereference that pointer in order to retrieve the value of
    "val" is undefined.
    It's also possible that whatever memory is stored at that location,
    contains a trap representation for "int". That's less likely than the
    pointer trap representation, but still possible. If so, that provides a
    third reason why the behavior of your program is undefined.
    f1() needs to have some way of transmitting the pointer that it gets
    from f2() back to main(). Here's the two best ways to do this:

    1: return the pointer as the value of f1():

    myStruct *f1(void)
    {
    myStruct *dst = f2();
    printf("dst->val=%d\n", dst->val);
    return dst;
    }

    in main():

    dst = f1();

    // after you're done with dst:
    free(dst);


    2. Pass a pointer to dst to f1(), and use that pointer to fill in dst:

    void f1(myStruct **pdst)
    {
    if(pdst == NULL)
    {
    // Insert error handling here
    return;
    }
    *pdst = f2();
    }

    in main():

    f1(&dst);

    // after you're done with dst:
    free(dst);
     
    James Kuyper, Feb 10, 2014
    #8
  9. wkevin

    Eric Sosman Guest

    (How can anyone "fix" it when we don't know what it's
    supposed to do? ...)

    Your problem: You haven't grasped what "pass by value"
    means. A useful way to think about it is to consider the
    function and its call separately:

    - The function has some number of /parameters/ of assorted
    types. Each parameter is very much like a local variable: It
    comes into existence when the function is called, and it ceases
    to exist when the function returns.

    - The function call has some number of /arguments/. These
    are expressions (sometimes rather trivial expressions) that are
    evaluated to produce values. The results -- not the expressions
    that produced them -- provide the initial values for the function's
    parameters for the invocation caused by the call.

    Got that? Argument expressions in the call produce values,
    the function parameters come into existence and are initialized
    with those values, the function executes and eventually returns,
    and the parameters disappear.

    So: If a function assigns a new value to a parameter, what
    happens to the function's caller?

    do do
    sol sol sol so-o-o-l ...
    do

    Now, now, put your answer in the form of a question ... That's
    correct: "What is Nothing At All?" is the right answer!

    When your function f1() assigns a new value to its parameter
    `dst', Nothing At All happens to the `dst' in main(), in either
    version. (The first version of main() had another problem: It
    used its own `dst' variable as an argument to initialize the `dst'
    parameter in f1(), which meant that main()'s `dst' had to be
    evaluated to get the value -- but main()'s `dst' had never been
    given any value, so its value was "indeterminate" (often called
    "garbage"), and even trying to evaluate it could get you in trouble.)

    If you're going to write C (actually, if you're going to write
    programs in any language I've ever encountered or even heard of),
    you need to acquire some level of understanding of the tool you're
    trying to use. Just fumbling around may eventually get you somewhere,
    but the chances of it getting you somewhere you want to be are slim.
     
    Eric Sosman, Feb 10, 2014
    #9
  10. wkevin

    wkevin Guest

    Hi,
    Thanks for your answer!
    The problem is that I cannot
    change f1(); it is a constraint which I have to adhere to!

    Is there some workaround to achieve my goal ?

    regards,
    Kevin
     
    wkevin, Feb 10, 2014
    #10
  11. wkevin

    James Kuyper Guest

    On 02/10/2014 04:56 PM, James Kuyper wrote:
    ....
    While technically true ;-}, that's not what I meant to say. Either
    remove the "not", or replace "no" with "a", whichever you prefer.
     
    James Kuyper, Feb 10, 2014
    #11
  12. wkevin

    Eric Sosman Guest

    Your goal being ...?
     
    Eric Sosman, Feb 10, 2014
    #12
  13. wkevin

    James Kuyper Guest

    I seriously doubt that. f1() contains several serious design flaws, as
    indicated in my previous message. If you can't fix those design flaws,
    there's only a limited number of pretty much useless things you can do
    with f1(), none of which are likely to be what you want to do. It can't
    have been written by a competent C programmer. If you can't modify it,
    you'll have to get the person who does have a right to modify it to fix
    those problems for you.
     
    James Kuyper, Feb 10, 2014
    #13
  14. wkevin

    BartC Guest

    You have to start using pointers to pointers then: eg. myStruct **dst as the
    parameter to f1(). Unless the constraint applies to that too?
     
    BartC, Feb 10, 2014
    #14
  15. wkevin

    Eric Sosman Guest

    If he can't change f1(), he can't change the parameter.
    (And if he changed the parameter as you suggest but could not
    change the statements inside the function, he'd get a compile-
    time error.)
     
    Eric Sosman, Feb 10, 2014
    #15
  16. wkevin

    Robbie Brown Guest

    int main(){
    myStruct *dst = f2();
    printf("dst->val in main=%d\n", dst->val);
    }

    no need to change f1()
     
    Robbie Brown, Feb 11, 2014
    #16
  17. wkevin

    James Kuyper Guest

    On 02/11/2014 12:25 PM, Robbie Brown wrote:
    ....
    He wasn't entirely clear on the matter, but I've been assuming that the
    constraint was not only that he couldn't change f1(), but that he was
    also required to meaningfully use it. Otherwise, the constraint would be
    meaningless - he would be free to create f3(), equivalent to f1(), but
    modified. That is, essentially, what you've done, except that you've put
    the code in main() instead of a separate function.
     
    James Kuyper, Feb 11, 2014
    #17
  18. wkevin

    Robbie Brown Guest

    Never assume anything ... or so my wizened old professor told me back in
    the mists of time.
    There are various ways of doing this aren't there. The two
    constraints/requirements that I got were 'can't change f1()' and
    'output 5 in main'. My solution requires the minimum of changes to
    achieve the required result. But you are right of course, we don't have
    the required background to be certain about anything. Still, given the
    information we *have* got I'm quite happy with my solution. I'd argue
    the point with the examiner but I defer to your (far) greater experience.
     
    Robbie Brown, Feb 11, 2014
    #18
  19. wkevin

    James Kuyper Guest

    On 02/11/2014 01:14 PM, Robbie Brown wrote:
    ....
    My experience suggests that something got garbled during the
    transmission of this constraint from whoever imposed it, through wkevin,
    to us. I'm not sure what got garbled, but something has to have been.
     
    James Kuyper, Feb 11, 2014
    #19
  20. Agreed. And in such cases, it's usually a waste of time to proceed
    without clarification from the person asking the question. (Often such
    clarification is not forthcoming, which leads me to the conclusion that
    it must not have been that important.)
     
    Keith Thompson, Feb 11, 2014
    #20
    1. Advertisements

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 (here). After that, you can post your question and our members will help you out.