Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion of error handling #3

Closed
elfring opened this issue Sep 5, 2013 · 6 comments
Closed

Completion of error handling #3

elfring opened this issue Sep 5, 2013 · 6 comments

Comments

@elfring
Copy link

elfring commented Sep 5, 2013

I have looked at a few source files for your current software. I have noticed that some checks for return codes are missing.

Would you like to add more error handling for return values from functions like the following?

@farindk
Copy link
Contributor

farindk commented Sep 5, 2013

Changed (and a bit more).
Thanks

2013/9/5 Markus Elfring notifications@github.com

I have looked at a few source files for your current software. I have
noticed that some checks for return codes are missing.

Would you like to add more error handling for return values from functions
like the following?

  • fopenhttp://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html=>
    mainhttps://github.com/strukturag/libde265/blob/aad31624ed7bdd3eb665511e569a8fb4e700525d/dec265/dec265.cc#L57
  • callochttp://pubs.opengroup.org/onlinepubs/9699919799/functions/calloc.html=>
    de265_new_decoderhttps://github.com/strukturag/libde265/blob/aad31624ed7bdd3eb665511e569a8fb4e700525d/libde265/de265.c#L65

Reply to this email directly or view it on GitHubhttps://github.com//issues/3
.

@elfring
Copy link
Author

elfring commented Sep 5, 2013

Thanks for your improvement.

I find it still incomplete.

Are you interested in aspect-oriented software development?

@farindk
Copy link
Contributor

farindk commented Sep 5, 2013

Fixed.

@farindk farindk closed this as completed Sep 5, 2013
@elfring
Copy link
Author

elfring commented Sep 6, 2013

The added value check is wrong. How do you think about an approach like the following?

   if (fread(buf, 1, BUFFER_SIZE, fh) != BUFFER_SIZE)
     {
      if ferror(fh)
         goto handle_read_error;
      else
        {
         ...
        }
     }

Do you prefer a different reaction than a crash because of a null pointer that might be returned from the function "de265_new_decoder"?

I guess that it would be nice if the function "de265_init" will acknowledge by a return value if the initialisation of your software library was really successful.

Would you like to care for return values also from functions like "fwrite" and "fclose"?

Would you like to add an annotation like "warn_unused_result" to any function of your API?

@farindk
Copy link
Contributor

farindk commented Sep 6, 2013

No, on contrary, your approach is wrong. Because at the end of a file, fread will return a value smaller than BUFFER_SIZE when it reads the final bytes.

I do not want to care about errors in fclose(). First, because that code is not used (only debugging), second, because I have never seen this fail, third, because I have far more important things to add.

@elfring
Copy link
Author

elfring commented Sep 6, 2013

You could add the check for "feof(fh)" in the else branch. ;-)

I hope that return value ignorance can be changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants