Completion of error handling

[elfring]

1. Would you like to add more error handling for return values from functions like the following?
   • pthread_mutex_lock ⇒ list_sort
   • pthread_spin_init ⇒ ht_set_list
2. How do you think about to annotate any functions with "__attribute__((warn_unused_result))"?

[xant]

 pthread_mutex_lock() will fail if:

 [EDEADLK]          A deadlock would occur if the thread blocked waiting for mutex.

 [EINVAL]           The value specified by mutex is invalid.

actually EDEADLK happens only if specific attributes are passed to PTHREAD_MUTEX_INIT (not our case) EINVAL only if the muxs is not valid which is not possible because initialization is checked (and memory corruption is not an option). So checking the return value of pthread_mutex_lock() is superfluous.

[jhi]

On Thursday-201508-27 14:30, Andrea Guzzo wrote:

|pthread_mutex_lock() will fail if: [EDEADLK] A deadlock would occur if the thread blocked waiting for mutex. [EINVAL] The value specified by mutex is invalid. |

actually EDEADLK happens only if specific attributes are passed to PTHREAD_MUTEX_INIT (not our case) EINVAL only if the muxs is not valid which is not possible because initialization is checked (and memory corruption is not an option). So checking the return value of pthread_mutex_lock() is superfluous.

You are assuming here a 100% compliant (nothing missing, nothing added, and completely forever unchanging) implementation. This is, usually, a fantasy. Just check the value.

— Reply to this email directly or view it on GitHub https://github.com/xant/libhl/issues/9#issuecomment-135515406.

[xant]

but are you really suggesting to always check the return value of pthread_mutex_lock() ? ... because this sounds quite unreasonable to me.

Or are we talking here about shutting up some specific static analyzer complaining about it ?

Because in this case , really, the only problem could be memory corruption, and it's a waste of time to protect from that (since it shouldn't happen and that's it)

[elfring]

I would prefer that strict error detection will become less optional in important software. Is the importance of software robustness increasing for the affected data structure library?

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

[jhi]

On Thursday-201508-27 14:46, Andrea Guzzo wrote:

but are you really suggesting to always check the return value of pthread_mutex_lock() ? ... because this sounds quite unreasonable to me.

Are you religiously opposed to checking return values?

[xant]

no ... and you can definitely notice it from the code. I just don't see why you should check pthread_mutex_lock() specifically. Apart memory corruption ... what could make it fail? Have you looked at other important software sourcecode? I have no problem in checking it everytime and I will probably do it. But I need to understand why it is necessary and why nobody else does it (since my understanding is that apart for memory corruption or some other specific behaviours there is no point in checking for the return value from pthread_mutex_lock())

so, apart from a blind approach like "everything must be checked" (even if you don't know what we are talking about) , is there any real reason here? do you always check for the return value from pthread_mutex_lock ? ... if yes (and it's not a recursive lock or a try_lock) why? (and maybe point to some example)

[jhi]

I just don't see why you should check pthread_mutex_lock() specifically.

I'm not saying one should check for pthread_mutex_lock() specifically.

I'm saying that if something can fail, one should check for it.

Arguing "but it cannot fail in this particular case" is in my opinion wasted effort (note also my earlier comment about implementations being imperfect), the effort is better spent in just adding the check and moving on.

There is this special biologist word we use for 'stable'. It is 'dead'. -- Jack Cohen

[xant]

so are you talking "generically" ? ... do you think I don't take into account that things can fail? :)

adding the check here means adding it everytime you call pthread_mutex_lock() ... so well ... what are we talking about?

[jhi]

so are you talking "generically" ? ... do you think I don' take into

account that things can fail? :)

Again, you are reading into my messages something I am not saying.

There is this special biologist word we use for 'stable'. It is 'dead'. -- Jack Cohen

[xant]

I'm just trying to understand what should be changed here. I agree with everything said/referred by elfring and I already wrapped allocations (since I understand that in certain environments you can't rely on allocators as you do generally on unix systems). But I still don't see how does this apply to pthread_mutex_lock() and if I should really check all calls to certain pthread functions ... which I think should require more explanations considering my previous points.

[elfring]

... and why nobody else does it ...

Can strict error detection become tedious in procedural programming languages like C? Would you like to look at relevant error predicates again in more detail?

There are some software development methodologies evolving which could help you here.

[xant]

my guess is that since that mutex is recursive his static analyzer complains about not checking the return value of pthread_mutex_lock() ... but in this case the only thing would be distinguishing between a memory corruption (bad ... your program should crash!) and a EDEADLK reported (which is the normal condition here) ... so if we want to change the code to shut up the static analyzer, just provide a patch for it or say which satic analyzer is complaining so I can test myself (instead of going ahead with riddles)

[xant]

elfring, can you point me out to some serious big software which actually checks all calls to the pthread interface (apart for specific reasons) ? Please ... or tell me why it is so important ... the change is trivial (but should be done everywhere) , error detection is not tedious but it has to make sense

[elfring]

I hope that open issues around a weakness 252 can also be reduced in this software library for basic data structures.

[xant]

lol :D .... now I know you are talking bullshit :) so the things are three here :

• or you are trying to run this library on some specific system and you need this kind of compliance (I would wonder what ... a space shuttle? :D) some details might help here
• or you want some stuff fixed because your static analyzer complains and you don't understand what it is talking about (output from the analyzer might help ...or even knowing which one)
• you are just trolling

[elfring]

... a space shuttle?

I hope that more complete API exception handling is not rocket science. ;-)

... because your static analyzer complains and you don't understand what it is talking about ...

I do understand my source code analysis well.

... trolling

I'm sorry when I hurt your software development feelings a bit.

[xant]

lol! you made my day ;)

please attach the output from the analyzer or point out some specific problem. Until then this issue is closed.

[elfring]

I guess that the concrete source code analysis tool does not really matter as long as it seems that you take corresponding suggestions personally. I hope that our different software development opinions can still be clarified for remaining update candidates in a constructive way a bit later.

[xant]

I just didn't get anything concrete about some sort of riddle game and reference to guidelines and other not-relevant stuff. It seems to me you are just wasting my time ... I tried getting out something useful from what you said and applied some patches ... but I can't see anything more apart a silly game of allusive questions and "opinions" not supported by actual code or real suggestions. We can leave opinions to how software development should be in general to another time and context. I think we could be a bit more specific here.

[elfring]

I suggest to follow good software development practices a bit more. Does the SEI CERT C Coding Standard matter for you?

[xant]

thanks, I'll follow your suggestion :) I'll also give a look at your code for reference ;)

[elfring]

A few update candidates are still left over:

• How do you think about to make the response for an error code configurable so that the user of your software library can choose if an other reaction is preferred than an abort() call after an error predicate check?
• Which reaction will be appropriate for a failed call of the function "malloc" (or "pthread_spin_init")?

[elfring]

Does a tool like "Coverity Scan" show any open issues which we discussed here?

[xant]

not anymore from what I saw yesterday after 6c07aefcb2e088e3e029ccc8e10d2221a0677c6c

[elfring]

Will it be interesting then to clarify the issues I mentioned before?