Thread: sending a struct to a function

  1. #1
    Registered User
    Join Date
    Dec 2007
    Posts
    84

    sending a struct to a function

    Hi Guys,

    I have the following code that is working fine and I was wondering what is the preffered way to send an array of struct to a function, and if I should make any changes to make this code look more proffestional.

    Code:
    #include <stdio.h>
    #include <stdlib.h>
    #include <conio.h>
    
    
    typedef struct
    {
      int day,month,year;
    } Date;
    
    
    typedef struct
    {
      Date date;
      int hh,mm;
      char description[100];
    
    } Meetings;
    
    
    void ShowMenu()
    {
      system("CLS");
      puts("a. Add");
      puts("d. Delete");
      puts("u. Update");
      puts("s. Show All");
      puts("x. Exit");
    }
    
    void AddMeeting(Meetings *m)
    {
      puts("Date:");
      scanf("%d/%d/%d",&m->date.day, &m->date.month, &m->date.year);
      puts("Hour:");
      scanf("%d:%d",&m->hh, &m->mm);
      flushall();
      puts("Description:");
      gets(m->description);
    }
    
    Meetings *AllocateMemory(Meetings *m, int *iTotal)
    {
      ++(*iTotal);
      m=realloc(m,(*iTotal)*sizeof(*m));
      if (m==NULL)
      {
    	puts("Not enough memory!");
    	exit(1);
      }
      return m;
    }
    
    int main(void)
    {
      Meetings *meetings=NULL;  
      int iTotal=0;
      char ch;
      
      do
      {
        ShowMenu();
    	ch=getch();
    
    	switch(ch)
    	{
    	  case 'a':
    		meetings=AllocateMemory(meetings,&iTotal);
    		AddMeeting(&meetings[iTotal-1]);
    	  break;
    	}
    
      } while (ch!='x');
    
      if (meetings!=NULL)
    	free(meetings);
    
      return 0;
    }
    Many Thanks!!!

  2. #2
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    > proffestional
    Spelling "professional" itself properly goes a long way.

    The only thing I'd say is that structures of any reasonably large size are generally passed by reference (with pointers) rather than by value, to avoid clobbering the stack (in other words, avoid taking lots of memory). You're already doing this, though, so no worries there.

    However, there's lots of other things wrong with your code.
    • gets() is bad, use fgets(). See the FAQ.
    • fflush(stdin) is bad. See the FAQ.
    • system("CLS") is not very good, either. See the FAQ.

    Consider not using the non-standard conio.h getch() as well. See the FAQ.

    "Where is this FAQ you speak of?" . . . http://faq.cprogramming.com/

    Also, free(NULL) is guaranteed to do nothing, so this
    Code:
      if (meetings!=NULL)
    	free(meetings);
    is redundant.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  3. #3
    Registered User
    Join Date
    Dec 2007
    Posts
    84
    Excellent words DWK!

    I have one more question.
    Which one of the following is the best way to pass the first element of the struct to a function?
    Code:
    1. AddMeeting(&meetings[0]);
    2. AddMeeting(meetings);
    3. AddMeeting(&meetings);
    Thanks Again!

  4. #4
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Well, the first two rely on a function taking a type* or type[] parameter, which is a good thing most of the time. The third one makes you take a parameter like type**, which, unless you're going to change where the pointer actually points to (rather rare), is just extra syntax.

    As for passing an array to a function -- I prefer array to &array[0]. It's shorter, and it sort of indicates to the reader that you're dealing with the whole array, not the first element of the array.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  5. #5
    Registered User
    Join Date
    Dec 2007
    Posts
    84
    Thank you again DWK!
    Good spelling and programming lesson!

  6. #6
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by salvadoravi View Post
    Excellent words DWK!

    I have one more question.
    Which one of the following is the best way to pass the first element of the struct to a function?
    Code:
    1. AddMeeting(&meetings[0]);
    2. AddMeeting(meetings);
    3. AddMeeting(&meetings);
    Thanks Again!
    The third is not equivalent to the first two. Of the first two, I would usually use the second. (If I saw the first in code, I would either think that (a) the person didn't know that meetings == &meetings[0], or (b) the person is explicitly stating that s/he only intends to deal with the first element of the array, depending on how competent the rest of the code looks and the comments nearby.) If you intend your function to process the whole array, you should certainly use form #2. I would only use form #1 if I really wanted to emphasize that I was only looking at element #0, or for parallelism with something like AddMeeting(&meetings[iTotal-1]) where the address-of-element format is clearly best.

    Either way, if your comments tell (accurately!) what's going on, I don't think anyone will care much about 1 vs. 2.

  7. #7
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Oh, you're welcome.

    A few other things:
    Code:
      puts("Date:");
      scanf("&#37;d/%d/%d",&m->date.day, &m->date.month, &m->date.year);
    You should specify what format you're expecting the date to be in -- dates can be written in many different ways. You might also consider using %*c instead of /'s in your scanf() format string -- %*c skips any character, so -'s or /'s or whatever would work.

    Code:
    m=realloc(m,(*iTotal)*sizeof(*m));
    That is generally a bad idea. If realloc() fails, it returns NULL, and you've lost your original block of memory that m was pointing to. It probably doesn't really matter in this case, but it's something you should keep in mind.

    In other words, don't use p=realloc(p,...) -- use a temporary variable, so that you can free p.
    Code:
    char *temp = realloc(p, ...);
    if(!temp) {
        free(p);
        perror("Out of memory");
        exit(1);
    }
    Oh, and checking for NULL from realloc() and malloc() is a good idea in any case.

    Notice perror() -- it can be quite useful at times. http://www.opengroup.org/onlinepubs/...ns/perror.html

    Excellent use of sizeof(*m), by the way. But you have to obfuscate it!
    Code:
    *iTotal*sizeof*m
    Just kidding.

    Your indentation is reasonably good, although it could be more consistent. It wouldn't pass an Elysia examination.
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  8. #8
    Registered User
    Join Date
    Dec 2007
    Posts
    84
    Quote Originally Posted by tabstop View Post
    If you intend your function to process the whole array, you should certainly use form #2. I would only use form #1 if I really wanted to emphasize that I was only looking at element #0, or for parallelism with something like AddMeeting(&meetings[iTotal-1]) where the address-of-element format is clearly best.
    What will be the preffered way:
    Code:
    1. AddMeeting(&meetings[iTotal-1]) 
    2. AddMeeting(&meetings+iTotal-1)

  9. #9
    Registered User
    Join Date
    Dec 2007
    Posts
    84
    Quote Originally Posted by dwks View Post
    Oh, you're welcome.
    Excellent use of sizeof(*m), by the way. But you have to obfuscate it!
    Code:
    *iTotal*sizeof*m
    Just kidding.
    I don't understand should I change sizeof(*m) to sizeof*m ??

    Thanks again

  10. #10
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by salvadoravi View Post
    What will be the preffered way:
    Code:
    1. AddMeeting(&meetings[iTotal-1]) 
    2. AddMeeting(&meetings+iTotal-1)
    Number 2 would have to be AddMeeting(meetings+iTotal-1); remember that meetings by itself is already a pointer, so &meetings is a pointer to a pointer and then it all gets weird.

    As to which is preferred: anyone who voluntarily uses #2 over #1 should be shot. (Well, maybe not really.)

  11. #11
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,336
    Quote Originally Posted by salvadoravi View Post
    I don't understand should I change sizeof(*m) to sizeof*m ??

    Thanks again
    No. Obfuscated can be interesting when you're in the mood for it, but for actual real code with actual real programs, no.

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by dwks View Post

    Also, free(NULL) is guaranteed to do nothing, so this
    Code:
      if (meetings!=NULL)
    	free(meetings);
    is redundant.
    That depends on how old the compiler / C runtime is. Many older compilers will not "ignore NULL" when called for free, which of course will mess up the entire memory allocation ever after. That said, it is a fair argument that if this doesn't work right, it's a good reason to consider ther compiler "ancient and in need of replacement with something new", just like a car with 500000 miles on the clock wouldn't be a good thing.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #13
    Frequently Quite Prolix dwks's Avatar
    Join Date
    Apr 2005
    Location
    Canada
    Posts
    8,057
    Yeah, allowing free(NULL) is part of C89, so any compiler that doesn't allow it must be very old indeed. http://www.cygwin.com/ml/gdb-patches.../msg00215.html
    dwk

    Seek and ye shall find. quaere et invenies.

    "Simplicity does not precede complexity, but follows it." -- Alan Perlis
    "Testing can only prove the presence of bugs, not their absence." -- Edsger Dijkstra
    "The only real mistake is the one from which we learn nothing." -- John Powell


    Other boards: DaniWeb, TPS
    Unofficial Wiki FAQ: cpwiki.sf.net

    My website: http://dwks.theprogrammingsite.com/
    Projects: codeform, xuni, atlantis, nort, etc.

  14. #14
    C++まいる!Cをこわせ!
    Join Date
    Oct 2007
    Location
    Inside my computer
    Posts
    24,654
    Quote Originally Posted by dwks View Post
    Your indentation is reasonably good, although it could be more consistent. It wouldn't pass an Elysia examination.
    You're right
    Your indentation is good, but it could be better! Good indentation is a good thing.
    Try not to mix spaces and tabs - it usually messes up code outside your editor! So use only tabs or only spaces. I recommend tabs.
    Also make the indentation consistent. Don't indent two spaces somewhere and a tab somewhere else. Just use one tab everywhere and everyone will be happy!
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  15. #15
    Registered User
    Join Date
    Dec 2007
    Posts
    84
    Quote Originally Posted by matsp View Post
    That depends on how old the compiler / C runtime is. Many older compilers will not "ignore NULL" when called for free, which of course will mess up the entire memory allocation ever after. That said, it is a fair argument that if this doesn't work right, it's a good reason to consider ther compiler "ancient and in need of replacement with something new", just like a car with 500000 miles on the clock wouldn't be a good thing.

    --
    Mats
    What is your recommendation?
    Should I use
    Code:
    if (meetings!=NULL)
       free(meetings);
    For portability?

    Or should I assume that anyone have new compiler?

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Including lib in a lib
    By bibiteinfo in forum C++ Programming
    Replies: 0
    Last Post: 02-07-2006, 02:28 PM
  2. Game Pointer Trouble?
    By Drahcir in forum C Programming
    Replies: 8
    Last Post: 02-04-2006, 02:53 AM
  3. Dikumud
    By maxorator in forum C++ Programming
    Replies: 1
    Last Post: 10-01-2005, 06:39 AM
  4. Problem with Visual C++ Object-Oriented Programming Book.
    By GameGenie in forum C++ Programming
    Replies: 9
    Last Post: 08-29-2005, 11:21 PM
  5. Request for comments
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 15
    Last Post: 01-02-2004, 10:33 AM