Thread: What is wrong with this program of passing structure into functions?

  1. #1
    Registered User
    Join Date
    Jan 2014
    Posts
    76

    What is wrong with this program of passing structure into functions?

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    void fun(struct node **x, struct node **y){
        struct node *k, *l;
        k=&(*x);
        l=&(*y);
        if(k->data>l->data)
            printf("true");
    else
        printf("false");
    }
    struct node(){
    int data;
    };
    void main(){
    struct *i, *j;
    int p=34, q=89;
    i=(struct i)malloc(sizeof(struct node));
    i->data=p;
    j=(struct i)malloc(sizeof(struct node));
    j->data=q;
    fun(&i,&j);
    getch();
    }
    Last edited by gaurav#; 08-08-2014 at 06:21 AM.

  2. #2
    Registered User
    Join Date
    Jun 2005
    Posts
    6,815
    If you make an effort to understand the error messages from your compiler, you will learn more than you would if someone simply summarised the (several) things wrong with your program.
    Right 98% of the time, and don't care about the other 3%.

    If I seem grumpy or unhelpful in reply to you, or tell you you need to demonstrate more effort before you can expect help, it is likely you deserve it. Suck it up, Buttercup, and read this, this, and this before posting again.

  3. #3
    Registered User
    Join Date
    Jun 2011
    Posts
    4,513
    Well, for starters, you should tell us what compiler warnings/errors are you receiving.

    I got quite a list from your code; here's the first two:

    Code:
    /*
    main.c|3|warning: 'struct node' declared inside parameter list|
    main.c|3|warning: its scope is only this definition or declaration, which is probably not what you want|
    */
    This is saying your struct declaration should go up top, before it is referenced in any code.

    Based on the various warnings and errors I received trying to compile your code, I get the impression you pretty much typed out all the code and only tried compiling it at the end.

    I'd recommend you scrap what you have and start over, but this time, only write a little bit of code, compile, fix any warnings/errors, and test. Then add a little more code, compile, fix any warnings/errors, and test. Build up your program gradually, compiling and testing as you go.

    I would suggested starting with only your struct above "main()", and in "main()", declare a struct variable, give it a value, and print it out. Compile and test this. When it works, then add a bit more code, following the advice in the previous paragraph.

    Also, the return type of "main()" should be int, not void. One of the proper forms of "main()" is as follows:

    Code:
    int main(void)
    {
        //
    
        return 0;
    }
    Link to FAQ: FAQ > main() / void main() / int main() / int main(void) / int main(int argc, char *argv[]) - Cprogramming.com

    And "getch()" is a non-standard function. Consider using "getchar()", or using other methods to keep the console window open.

    Link to FAQ: FAQ > Stop my Windows Console from disappearing everytime I run my program? - Cprogramming.com

  4. #4
    Registered User zub's Avatar
    Join Date
    May 2014
    Location
    Russia
    Posts
    104
    OMG. One of the ugliest programs I've seen in my life.

    Code:
    #include <stdio.h>      // puts
    #include <stdlib.h>     // malloc
    
    
    struct node {
        int data;
    };
    
    
    int fun(const struct node* const x, const struct node* const y)
    {
        return x->data > y->data;
    }
    
    
    int main(void)
    {
        struct node* i = (struct node*) malloc(sizeof(struct node));
        i->data = 34;
        struct node* j = (struct node*) malloc(sizeof(struct node));
        j->data = 89;
        if( fun(i, j) ) {
            puts("true");
        } else {
            puts("false");
        }
        return 0;
    }
    Last edited by zub; 08-08-2014 at 01:12 PM.
    Our goals are clear, tasks are defined! Let's work, comrades! -- Nikita Khrushchev

  5. #5
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    zub: you might want to explain your changes instead of just posting code.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  6. #6
    Registered User zub's Avatar
    Join Date
    May 2014
    Location
    Russia
    Posts
    104
    I find it easier to write code than to explain, because English is not my native language. Moreover, in this particular case, no explanation needed. Compare line by line and you will understand what I have done fixes.

    I can further simplify the code with a few useful tricks. Good code is self-evident and self-documenting.

    By the way, I forgot to free memory. All the time I forget it, because my own projects use my own allocator, which does not require this.

    Code:
    #include <stdio.h>      // puts
    #include <stdlib.h>     // malloc
    
    
    #define NEW(x) ((x)malloc(sizeof(struct x)))
    
    
    typedef struct Node {
        int data;
    } *Node;
    
    
    int fun(const Node const x, const Node const y)
    {
        return x->data > y->data;
    }
    
    
    int main(void)
    {
        Node i = NEW(Node);
        i->data = 34;
        Node j = NEW(Node);
        j->data = 89;
        if( fun(i, j) ) {
            puts("true");
        } else {
            puts("false");
        }
        free(i);
        free(j);
        return 0;
    }
    Our goals are clear, tasks are defined! Let's work, comrades! -- Nikita Khrushchev

  7. #7
    Registered User
    Join Date
    Jun 2011
    Posts
    4,513

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by zub
    I find it easier to write code than to explain, because English is not my native language.
    Ah. Don't worry, the idea is to highlight things that the the help seeker might miss. You don't have to have perfect English to do that.

    Quote Originally Posted by zub
    Moreover, in this particular case, no explanation needed. Compare line by line and you will understand what I have done fixes.
    It is not always reasonable to expect a newbie to be able to distinguish between changes that are substantially important, those that are important stylistically, and those that are merely subjective style, just by reading code.

    Quote Originally Posted by zub
    I can further simplify the code with a few useful tricks. Good code is self-evident and self-documenting.
    Unfortunately, your typedef of struct Node* to be Node makes code involving Node no longer self-evident, and your NEW function-style macro is not self-documenting since it relies on the macro argument being of a struct type. Compare:
    Code:
    Node i = NEW(Node);
    i->data = 34;
    with:
    Code:
    struct Node *i = malloc(sizeof(*i));
    i->data = 34;
    In the former, one must already be familiar with the NEW macro and know that for NEW(Node) to work the type struct Node must exist; accessing i->data is surprising since Node does not appear to be a pointer. In the latter, the reader is likely to be familiar with malloc and sizeof, and i->data is not surprising since i is obviously a pointer.

    Instead of a NEW macro, I would suggest a function for creating and initialising the node, e.g.,
    Code:
    struct Node *node_init(int data)
    {
        struct Node *node = malloc(sizeof(*node));
        if (node)
        {
            node->data = data;
        }
        return node;
    }
    This can then be called in main with no loss of clarity, e.g.,
    Code:
    struct Node *i = node_init(34);
    Quote Originally Posted by zub
    By the way, I forgot to free memory. All the time I forget it, because my own projects use my own allocator, which does not require this.
    That's fine: gaurav#'s code made the same mistake, and we are certainly not responsible for pointing out all possible mistakes in code posted by someone asking for help.
    Last edited by laserlight; 08-08-2014 at 01:59 PM.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Registered User Alpo's Avatar
    Join Date
    Apr 2014
    Posts
    877
    Quote Originally Posted by zub View Post
    Compare line by line and you will understand what I have done fixes.

    I can further simplify the code with a few useful tricks. Good code is self-evident and self-documenting.
    This is true sometimes, but it is very hard to judge in regards to a person reviewing there own code. It is an easy logic trap to fall into, to believe that what you are doing is evident. It relies on the assumption that the person reading the code will have all the same associations as yourself though.

    For instance, when you read the code it might seem natural to treat Node as a pointer because you have made the association that that's just what Node is. Another person reading or debugging it might have to continually look up the definitions for that variable to make sure they are treating it as the correct type.

    Edit: Whoops, wasn't trying to parrot you LaserLight (great minds and all that? No? ahh well :P)
    Last edited by Alpo; 08-08-2014 at 10:22 PM.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Passing structure by reference through 4 functions
    By nickdavid35 in forum C Programming
    Replies: 14
    Last Post: 04-30-2010, 09:39 AM
  2. Passing Structure Pointers to Functions
    By samus250 in forum C Programming
    Replies: 15
    Last Post: 03-20-2008, 03:13 PM
  3. passing structure arrays to functions?
    By bem82 in forum C Programming
    Replies: 3
    Last Post: 10-30-2006, 06:17 AM
  4. structure question: passing to functions
    By kocika73 in forum C Programming
    Replies: 4
    Last Post: 03-07-2005, 08:58 PM
  5. Replies: 1
    Last Post: 01-20-2002, 11:50 AM