Project

General

Profile

Actions

Bug #99

closed

Check return codes everywhere

Added by Markus Elfring almost 14 years ago. Updated about 13 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
% Done:

0%

Source:
Tags:
Backport:
Regression:
Severity:
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

This issue corresponds to my previous "feature request".

Some checks for return codes are missing.

Examples:
Would you like to add more error handling for return values from "strdup" like in the function "ceph_set_default_id" and from "write" in the function "do_cli"?
source:src/config.cc@a31ea46f35a3e49b5565c88522b27ba3538147d2#L56
source:src/ceph.cc@d2d80c6e80c69655156f6f1cf7f724a7b36f90c3#L462

Actions #1

Updated by Sage Weil almost 14 years ago

Yeah, there are lots of these. Some hosts seem to spit out nice warnings for lots of call sites where the return value isn't checked (newer glibc i think?). It would be great to clean these up. Some care needs to be taken, though: in some cases failure is okay, in some cases a warning is appropriate, in others we should outright fail.

Actions #2

Updated by Markus Elfring almost 14 years ago

I suggest to avoid unchecked function calls.
Would you like to detect every error situation as early as possible?

Would you like to reduce the efforts for error code checking by an exception class hierarchy?

Actions #3

Updated by Sage Weil almost 14 years ago

For something like strdup, which fails with ENOMEM, we can throw the usual C++ out of memory exception.

In general, we should check errors right away. If it's ENOMEM, throw an exception. If it's non-fatal, warn. If we can, pass the error to the caller. Otherwise... it needs a closer look! assert(r == 0) as a last resort.

Actions #4

Updated by Markus Elfring almost 14 years ago

Would you like to reuse any class library?

I do not like "assert" for consistent error handling because the check will not be performed if the symbol "NDEBUG" will be defined.

I would appreciate a clarification which function calls need to be kept as C style
interfaces to provide the portable ABI and which of them could use more C++ features
for their implementations.

Are there any chances to integrate higher level programming tools like AspectC++ and ACC into the software development process here?

Actions #5

Updated by Sage Weil almost 14 years ago

Markus Elfring wrote:

Would you like to reuse any class library?

I do not like "assert" for consistent error handling because the check will not be performed if the symbol "NDEBUG" will be defined.

Not actually with ceph's src/include/assert.h. If it did, we'd need to keep some other 'die' helper to replace instances of assert(0) all over the code base.

I would appreciate a clarification which function calls need to be kept as C style
interfaces to provide the portable ABI and which of them could use more C++ features
for their implementations.

Are there any chances to integrate higher level programming tools like AspectC++ and ACC into the software development process here?

I have no specific rules here. I'd like to avoid too many build dependencies, though. We already use boost, so anything there is probably worthwhile.

But I wouldn't spend too much time fixing things that aren't broken, unless the existing approach is somehow not robust or it is making error handling harder or something.

Actions #6

Updated by Markus Elfring almost 14 years ago

The C/C++ programming language makes it easy to overlook unused return values. Which software tools do you find acceptable for the purpose to handle them all?

Actions #7

Updated by Sage Weil about 13 years ago

  • Status changed from New to Closed
Actions

Also available in: Atom PDF