Completion of error handling

[elfring]

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

• malloc ⇒ papplJobFilterImage
• pthread_mutex_lock ⇒ _papplDNSSDInit
• strdup ⇒ copy_link

[michaelrsweet]

I will look at adding some checks for the two memory allocations. The calls are unlikely to fail with the checks that are already in place, and its not like the links are some unbounded values being passed in from a third-party, they are explicitly added by the printer application developer.

WRT the mutex call, given the two possible documented errors (not initialized or deadlock) and the code in question it will impossible for the mutex to fail unless the data segment gets trashed, at which point a lot more things will be failing as well... Moreover, there isn't any meaningful way to deal with a failure, if one did occur - deadlock means the current thread already has a lock (no problem), and not initialized means something bad has happened and we'll be crashing real soon now.

[elfring]

I suggest to avoid ignorance of return values a bit more. Would you like to detect every error situation as early as possible?

[michaelrsweet]

Not if there is nothing useful that can be done in response to the error.

[michaelrsweet]

(And no, I do not subscribe to the programmer community that believes that calling abort under such circumstances is appropriate.)

[elfring]

There are the usual possibilities to consider for more complete error reporting.

How do you think about to improve static source code analysis also for this software?

[michaelrsweet]

@elfring I've been using the Clang static analyzer as well as LGTM on this project from day one. I tried Codacy but it is way too noisy (particularly the cppcheck output) and no way to configure things... :/

[elfring]

I find it interesting then that I can find remaining open issues by simple source code searches.

Would you like to clarify further software development possibilities (also for this issue)?

[michaelrsweet]

@elfring Please be specific. With all due respect, you have a habit of making vague bug reports about issues you are concerned about, but I need specific details (files, line numbers, and issues) or the output from one of the static analysis tools you are using to find the issues.

[elfring]

I am used to a selection of source code search patterns. There are further means available for better error detection and corresponding exception handling.

• Annotations
• Would you become interested to apply aspect-oriented software development?

[michaelrsweet]

@elfring I am not interested in adding yet another layer/dependency to the PAPPL sources, so Aspect C/C++ is not of interest to me.

[elfring]

How do you think about to extend your selection of software analysis tools according to presented issues?

[michaelrsweet]

@elfring I have managed to add Cppcheck to PAPPL master, and it is now part of the Travis CI workflow. I've reviewed all of its reported issues using the CERT checker, however the MISRA, threading, and Y2038 add-ons produced such an incredible volume of false-positives that I opted to not enable them. Unfortunately, inline suppression comments don't appear to work so one of the suppressions I am using for the CERT warnings (for casting const pointers to non-const) has to be a little more aggressive than I'd like.

Frama-C also looks promising but seems to have serious problems parsing the system header files... The documentation is also a little obtuse...

[elfring]

According to error detection for POSIX API functions:

• I tried to extend the analysis tool “Cppcheck” for this purpose.
• I constructed scripts for the semantic patch language for showing unstored return values.
• Will you care more for standard function calls?

[michaelrsweet]

@elfring I will happily review a list of issues (source file, line number, and issue description) or a pull request with your proposed changes. My goals and priorities for the PAPPL project may not be the same as yours, but I will incorporate any changes that make sense to me.

[michaelrsweet]

OK, I've done a review of all calloc, malloc, and strdup usage and made appropriate changes as needed. If you find any other cases where the return value of a function call needs to be checked but isn't, please file new issues with the specific areas of concern.

[elfring]

OK, I've done a review of all calloc, malloc, and strdup usage and made appropriate changes as needed.

Thanks for a few source code adjustments.

If you find any other cases where the return value of a function call needs to be checked but isn't,

:thinking: You know already that further return values are overlooked so far.

please file new issues with the specific areas of concern.

Do you really want to get separate bug reports for each missed error code check?

Please reopen this issue accordingly.

[michaelrsweet]

@elfring I want bug reports for specific issues. Ideally I'd like the issues grouped by logical category, but you can submit a single bug report listing all of the specific issues you think need to be addressed.

I am not re-opening this bug report - I have updated all of the unchecked memory allocations that needed to be checked. Since today's code functions without (recoverable) errors on a resource-constrained embedded Linux controller while getting hammered by thousands of simultaneous client requests, I am not prepared to make any changes to the pthread calls today. Aborting on any error could cause other random, impossible to reproduce crashes causing irreversible data loss and/or denial-of-service.

When I end up porting PAPPL to Windows (#116) I'll need to provide threading primitive "wrapper" functions, at which point I can implement something a bit smarter for EAGAIN and EDEADLK errors from the pthread calls and abort for other errors that "should never happen". That would be marginally better than what exists today, and I've included a pointer back to this issue so that I don't forget...