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?

• fclose ⇒ print_qr_code
• printf ⇒ print_device
• pthread_mutex_lock ⇒ shared_policy_reload

[zonque]

We never check for return values of printf(), why should we do it in print_device()?

In print_qr_code(), I don't see how fclose() could possibly fail? That said, I figure the code there could make use of an _cleanup_fclose_ annotation.

The pthread_mutex_lock() call in shared_policy_reload() could be wrapped in an assert_se().

Feel free to open pull requests for the latter two, and for similar cases if you find any.

[elfring]

I suggest to avoid ignorance of return values a bit more.

Are you interested to apply aspect-oriented software development?
How do you think about to encapsulate error detection and corresponding exception handling as a reusable aspect in your software?

[zonque]

We do pay attention to return values wherever it really makes sense. I don't see how that applies to printf() though. What sort of exception handling do you have in mind for software written in C? I can't quite follow.

Disregard my earlier comment on print_qr_code(), btw. The file needs to be closed before the buffer is accessed, so there is no simpler way. If you explain why fclose() can fail even though ferror() signaled success before, let's discuss the details in a PR.

[elfring]

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

Do you find information sources like the following useful?

• Article "A Reusable Aspect for Memory Allocation Checking"
• Document "Aspect-Oriented Software Development - An Introduction" by Johan Brichau and Theo D'Hondt
• Paper "On Typesafe Aspect Implementations in C++" by Daniel Lohmann and Olaf Spinczyk

[zonque]

Your first two links point to non-existent content. The latter is about C++, so it's not applicable to the system project, which is written in C.

We do run static checkers on the code base on a nightly base:

https://scan.coverity.com/projects/350/
https://systemd.github.io/systemd-build-scan/

The latter produces lots of false positives though, as LLVM does not take our automatic cleanup annotations into account. If that could be improved, I'm happy to tweak the scripts.

Any type of automated checking and ways of improving the software is generally interesting. But please stick to suggestions that apply to the goals of the software project at hand.

[elfring]

A tool like AspectC++ can also be reused for source files written in the C programming language.
Would you dare to delegate any software generation tasks to such a dedicated tool?

[zonque]

No, AFAIK, we refrain from using software generation tools. And I can't quite follow what you're suggesting. Could you maybe post to our ML, describing what you have in mind, and providing some easy to understand examples? Thanks!

[elfring]

Are you used to the identification of cross-cutting concerns?

Would you eventually prefer to achieve corresponding improvements by the Coccinelle software instead?

[zonque]

If you want to provide semantic Coccinelle patches for common patterns of mistakes that you spot, please collect them in a new repository. We are happy to link to it, as long as it is maintained, or even host it under the umbrella of our GitHub organization. Not sure if those patches would be very specific to systemd though.

[elfring]

How do you think about to extend another semantic patch approach?

[zonque]

I'm really lost in parsing your cryptic posts, sorry.

Please provide concrete examples of what you have in mind, specifically for this project. Open a PR containing your changes, so we have a look. Or post to the ML, providing comprehensive examples.

Thanks.

[elfring]

I suggest to look for recurring patterns in source files. It seems that there are some open issues left over despite of the reuse of two popular static source code analysis tools here.

Dedicated software development tools can help you more than you might know at the moment, can't they?

[zonque]

Tools are generally welcome. Please use them and provide patches. If possible, we should hook them up so they run automatically, in which case any help of achieving that is appreciated.

[elfring]

If a higher level patch format would be reused, further update suggestions could be applied to a wider range of source files to some degree automatically.
How do you think about to reduce specific manual software maintenance tasks?

[mbiebl]

@zonque sounds (almost) like a bot to me

[elfring]

@mbiebl: Would you like to continue a constructive discussion around update candidates in this software?