Completion of error handling

[elfring]

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?

• fwrite ⇒ get
• close ⇒ sparkey_hash_close

[spkrka]

For the first case I agree, there should be some error handling for fwrite in get in main.c

As for sparkey_hash_close, I am not sure what a suitable way to handle that error would be. Closing a read-only file should always succeed and be a safe operation.

If you have any convincing arguments or a good patch, I'll be happy to reevaluate my position on it.

[spkrka]

Is this an improvement for the first of your issues at least? 1c499c0

[elfring]

Yes, of course. - How do you think about to rename the added function so that its functionality can be easily distinguished from the well-known macro "assert"?

I hope that return value ignorance can be changed further. Would you like to add an annotation like "warn_unused_result" to any function of your API?

[spkrka]

Sure, the assert could be renamed, but it's not part of the public api so does it really matter?
I guess warn_unused_result would make sense for almost everything in the API.

Feel free to make pull requests if there's something you want to fix, I'll try to review and merge as soon as possible.

[elfring]

I suggest a small disambiguation. Would you like to add a cast to void for the error message there (and similar places)?

[spkrka]

I am not sure why we need that. Are you running some sort of extra strict compiler or compilation flags or analysis tool?

If your endgoal is to reduce false positives or build failures, that would make perfect sense, but then it's probably best if the developer who is using such a compiler or tool makes the suitable change to fix it, otherwise it would mostly be fumbling in the dark. Your change sounds good overall, but I want to be able to reproduce the problem you're having before writing any fix.

[elfring]

I would like to make static source code analysis easier and a bit more complete.

Are you interested in aspect-oriented software development?

[spkrka]

I am still not quite understanding your issue.

Do you want to use sparkey, but your static source code analysis finds errors so you can't do that? If so, can you tell me how you're trying to build it so I can reproduce the errors or fix them? Alternatively you can fork and fix them yourselves and submit a pull request.

Are you building a static source code analyser and are using sparkey as sample input? If so, I think it would be easiest if you forked and fixed the easy errors you find and create separate issues for each non-trivial error, including instructions for how to get that error.

[elfring]

I have found update candidates during my own little code review. Now I try to clarify if fixes should be applied manually or if improvements can also be achieved by specific tools automatically.

Error detection (and corresponding exception handling) is a well-known cross-cutting concern. How do you think about to encapsulate it as a reusable aspect in your software?

[rohansingh]

Are any further changes going to come out of this issue? I don't see checking for errors on close, nor casting things to void, actually happening. Tempted to close this.

@elfring Do you have any other specific changes for or issues with sparkey?

[elfring]

It seems that the Sparkey software was already strong with regarding of return values. There might be a few places "left over" for further improvements. ;-)

[spkrka]

Closing the issue, as there is nothing really specific that should be done.