need help on error handling.

This is a discussion on need help on error handling. within the C Programming forums, part of the General Programming Boards category; Hi, I need some serious suggestions about error handling in C. Can some one please tell me of a better ...

  1. #1
    Registered User
    Join Date
    Apr 2008
    Posts
    87

    need help on error handling.

    Hi, I need some serious suggestions about error handling in C. Can some one please tell me of a better method to handle errors in my project. I will just post some code that I have written:

    Code:
    int init_plwave(void)
    {
      int flag = 0;
      double theta, phi;
      double freq;
      double e0;
      unsigned long nrays;
    
      printf("Enter the number of rays\n");
      if(scanf("%lu", &nrays) != 1)
      {
        fprintf(stderr, "Error while reading the number of rays\n");
        flag = 1;
        return flag;
      }
    
      printf("Enter the frequency of the rays\n");
      if(scanf("%le", &freq) != 1)
      {
        fprintf(stderr, "Error while reading the frequency of the plane wave\n");
        flag = 1;
        return flag;
      }
      
      printf("Enter the direction(theta, phi) for the rays\n");
      if(scanf("%lf %lf", &theta, &phi) != 2)
      {
        fprintf(stderr, "Error while reading the direction for the rays\n");
        flag = 1;
        return flag;
      }
    
      printf("Enter the source electric field\n");
      if(scanf("%lf", &e0) != 1)
      {
        fprintf(stderr, "Error while reading the source electric field\n");
        flag = 1;
        return flag;
      }
      return flag;
    }
    
    int read_mesh(char *fname, mesh **m)
    {
      FILE *fp;
      int flag = 0;
    
      fp = fopen(fname, "r");
      if(fp == NULL)
      {
        fprintf(stderr, "Error while opening file %s\n", fname);
        flag = 1;
        return flag;
      }
    
      *m = malloc(sizeof(mesh));
      if(*m == NULL)
      {
        fprintf(stderr, "Couldn't allocate memory for the mesh\n");
        flag = 1;
        return flag;
      }
    
      if(parse_zeus_file(fp, m))
      {
        flag = 1;
        return flag;
      }
    
      return flag;
    }
    Although, I do not feel there is anything wrong with this approach of checking for the error condition and then using fprintf to print errors, return some flag and terminate the function but as you can all see it is extremely verbose, repetitive and it looks as if there is hardly any code but only error handling in the program. Looks really ugly and gives a bad impression. I need some idea on a centralized error handling appraoch or atleast some suggestions on improving my technique. please help me.
    Last edited by broli86; 06-19-2008 at 10:44 AM.

  2. #2
    Technical Lead QuantumPete's Avatar
    Join Date
    Aug 2007
    Location
    London, UK
    Posts
    894
    Ah pet peeve of mine
    Rather than checking whether the function returns a non_zero rcode, assign the return value of the function to a variable (called rcode or something) and test that. Then your printf statement can print out what the error was.
    Then, instead of always returning 1, return a different code for each condition. The calling function should then print out that rcode and name of the function. This way, you basically get a stack trace of error codes and it will be very easy to pinpoint where the problem is.
    Never use the same error message in two different places (I know you've not done that, but it's good practice).
    If your code needs to abort because of an error (such as not being able to allocate enough memory), you don't need to set a flag and then return that flag. Just return the error code for that condition. If, however, you have a function that can soldier on, even when something has gone wrong, you can use flags to indicate at the end of the function that something went wrong, but it wasn't critical. You'd want to print something like "There was an error (code x), function_name recovered and terminated normally".

    The reason your code looks verbose and ugly is because there is very little actual implementation code. Once you flesh out what you're doing, you'll see that in most cases the normal code far outweighs error handling.

    Hope that helps,

    QuantumPete
    "No-one else has reported this problem, you're either crazy or a liar" - Dogbert Technical Support
    "Have you tried turning it off and on again?" - The IT Crowd

  3. #3
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    Quote Originally Posted by QuantumPete View Post
    Ah pet peeve of mine
    Rather than checking whether the function returns a non_zero rcode, assign the return value of the function to a variable (called rcode or something) and test that. Then your printf statement can print out what the error was.
    Then, instead of always returning 1, return a different code for each condition. The calling function should then print out that rcode and name of the function. This way, you basically get a stack trace of error codes and it will be very easy to pinpoint where the problem is.
    Never use the same error message in two different places (I know you've not done that, but it's good practice).
    If your code needs to abort because of an error (such as not being able to allocate enough memory), you don't need to set a flag and then return that flag. Just return the error code for that condition. If, however, you have a function that can soldier on, even when something has gone wrong, you can use flags to indicate at the end of the function that something went wrong, but it wasn't critical. You'd want to print something like "There was an error (code x), function_name recovered and terminated normally".

    The reason your code looks verbose and ugly is because there is very little actual implementation code. Once you flesh out what you're doing, you'll see that in most cases the normal code far outweighs error handling.

    Hope that helps,

    QuantumPete
    Do you think using __LINE__ , __FILE__ and __func__ macros can help ? For eg.

    Code:
    int something(void)
    {
      double *b;
      int flag = 0;
    
      b = malloc(sizeof(double) * 500);
      if(b == NULL)
     {
        fprintf(stderr, "Memory allocation failure: module %s line %d function %s", __FILE__, __LINE__, __func__);
       flag = 1;
       return flag;
     }
    
      return flag;
    }

  4. #4
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,180
    Yes, that's a good thing in helping localize where the problem occurred.
    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.
    For information on how to enable C++11 on your compiler, look here.
    よく聞くがいい!私は天才だからね! ^_^

  5. #5
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    Quote Originally Posted by Elysia View Post
    Yes, that's a good thing in helping localize where the problem occurred.
    Is this really a debugging stratergy or error handling ?

  6. #6
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,180
    It's more of debugging nature. It's priceless when shipping an app to someone else and it goes wrong somewhere.
    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.
    For information on how to enable C++11 on your compiler, look here.
    よく聞くがいい!私は天才だからね! ^_^

  7. #7
    Registered User slingerland3g's Avatar
    Join Date
    Jan 2008
    Location
    Seattle
    Posts
    602
    Using assert() may be more useful to check conditionals.

  8. #8
    Registered User
    Join Date
    Apr 2008
    Posts
    87
    Quote Originally Posted by slingerland3g View Post
    Using assert() may be more useful to check conditionals.
    But assert will terminate the program.

  9. #9
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,180
    assert is usually for programmer errors that are code errors, so to speak.
    If it isn't - if something goes wrong, for example, then it usually is a better idea not to use assert.
    assert is good for NULL pointers, for example.
    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.
    For information on how to enable C++11 on your compiler, look here.
    よく聞くがいい!私は天才だからね! ^_^

  10. #10
    Registered User slingerland3g's Avatar
    Join Date
    Jan 2008
    Location
    Seattle
    Posts
    602
    I agree that, if this is for debugging, there are other means, but for simple error checking and allowing the user to either change their input and try again will be a better approach.

    Is the intended purpose to have the program limp along for the user and latter crash?

Popular pages Recent additions subscribe to a feed

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21