Github user dcristoloveanu commented on the pull request:
    Hi Andrew,
    Thanks for the review, it's much appreciated.
    There are several good points here that deserve a more in depth discussion, 
so I will attempt to address them one by one:
    1. "Using LINE as an error is not acceptable"
    I typically use __LINE__ as return code when it does not really matter why 
the function failed, and the caller does not really need to check which error 
case what hit. I can understand that's not the case for Proton, and I can 
change this to using an existing error code. I simply underestimated that this 
is a big deal, so I'll switch to your suggestion.
    2. "The Proton code does not subscribe to the practice of forcing a single 
return to a routine."
    I can understand the choice, and I shall respect that, living by the "when 
in Rome" rule.
    On the other hand, as an FYI, MISRA for example has an advisory rule (15.5) 
that states "A function should have a single point of exit at the end". One of 
the rationale is that if a function has exit points interspersed with 
statements that produce persistent side effects, it is not easy to determine 
which side effects will occur when the function is executed.
    Anyhow, this is like I said just an FYI remark, I do suspect that you've 
already considered the plus/minuses for single vs. multiple return point and  
this can quickly turn into a not so productive discussion.
    3. "On the whole I feel that this kind of error situation should be dealt 
with using an exception like mechanism. "
    On this one I respectfully disagree. I do not think it is for Proton to 
decide whether the system should be brought down or not. It is at system level 
that such a decision is made. For example one could have on an embedded device 
a special memory management implementation that would decide that if malloc 
fails a reboot function should be invoked (or any other action taken). But 
Proton has no business of taking such a decision for an application, since 
Proton is merely a protocol library. Should for example openssl crash the 
system if it cannot allocate a block?  I believe not, since it's not openssl's 
responsibility either to decide that.
    So it follows that the ideal action for a library like openssl, proton, 
etc. in case an error happened is to report the errors to the user in a way 
that the user can act on them, without leaving the library in an unusable state 
for the next operation, if a next operation would be possible.
    An exception-like mechanism would require the state to be unwound, which is 
OK in other languages, but C's longjmp is not really good for that. So the only 
solution that I believe is left is to report the errors as return codes until 
the point they are surfaces to the consuming application (as return codes or 
tracker status), then the application which can decide for itself whether to 
bring down the system, print an error or set the building on fire :-).
    I don't think ignoring the errors is acceptable if Proton is to considered 
for usage in many embedded environments (like automotive, industrial, etc. - 
MISRA is a requirement in many such cases), that's why I submitted this PR.
    I'll wait for your thoughts before submitting an update to the PR.
    Also, as a side point, I could go and fix this throughout the code base, 
but I fear there would be a lot of changes, thus I opted for fixing things one 
by one so it gets a little better with each PR. If you think we should do this 
in a different way, let me know, any guidance is appreciated.

If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at or file a JIRA ticket
with INFRA.

Reply via email to