Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Completion of error handling #38

Closed
elfring opened this issue Oct 22, 2014 · 19 comments
Closed

Completion of error handling #38

elfring opened this issue Oct 22, 2014 · 19 comments
Assignees
Milestone

Comments

@elfring
Copy link

elfring commented Oct 22, 2014

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?

@droe droe self-assigned this Oct 22, 2014
@droe droe added the bug label Oct 22, 2014
@droe
Copy link
Owner

droe commented Oct 22, 2014

Makes sense; feel free to submit patches or send pull requests.

@elfring
Copy link
Author

elfring commented Oct 23, 2014

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?

@droe
Copy link
Owner

droe commented Oct 23, 2014

SSLsplit was written to scratch an itch, not to win a software engineering competition :)

I do appreciate specific bug reports about unchecked error conditions and I will eventually work through them; in the meantime you are free to submit patches.

@elfring
Copy link
Author

elfring commented Oct 23, 2014

I try to check the corresponding change acceptance before increasing software development efforts for a preferred solution.

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

Do you find information sources like the following useful?

@droe
Copy link
Owner

droe commented Oct 23, 2014

I do not intend to introduce AoP to this project. I do welcome specific proposals to improve encapsulation, separation of concerns etc within the plain c codebase.

Regarding static analysis, have you seen the lint target of the current make file?

@elfring
Copy link
Author

elfring commented Oct 23, 2014

I guess that some more time will need to pass by until aspect-oriented development might become a general part also for your software design and maintenance.

The tool "cppcheck" can help to find some interesting issues in source code. But I find that it will not be useful to improve the situation for unused (or overlooked) return values at the moment.
I find other approaches more promising.

@droe
Copy link
Owner

droe commented Oct 23, 2014

By all means, feel free to run more promising approaches against the code base and submit the resulting patches.

@elfring
Copy link
Author

elfring commented Oct 23, 2014

How promising do you find tools like the following for suggested source code improvements?

  • AspectC++
  • Coccinelle

droe added a commit that referenced this issue Oct 23, 2014
droe added a commit that referenced this issue Oct 23, 2014
Issue:		#38
Reported by:	Markus Elfring
@droe
Copy link
Owner

droe commented Oct 23, 2014

Fixed bullet points 2 and 3. Not fixing bullet point 1 because if fprintf(stderr) fails at that point then there is no useful thing to do anyway.

@droe droe closed this as completed Oct 23, 2014
@droe droe added this to the 0.4.9 milestone Oct 23, 2014
@elfring
Copy link
Author

elfring commented Oct 23, 2014

Would you like to inform your program user by the exit status code that a requested file output failed eventually?

@droe
Copy link
Owner

droe commented Oct 23, 2014

I'm not opposed to accepting a patch for that if somebody cares about knowing if -V failed.

@droe droe added the released label Apr 23, 2015
@elfring
Copy link
Author

elfring commented Mar 28, 2016

Would you like to look once more at any remaining update candidates?

@droe
Copy link
Owner

droe commented Mar 28, 2016

I will commit a fix for the first one, thanks. The second one is bogus; if strdup() fails, it returns NULL, which means sys_user_str() returns NULL, which in turn signals an error to the caller.

droe added a commit that referenced this issue Mar 28, 2016
Issue:		#38
Reported by:	Markus Elfring
@elfring
Copy link
Author

elfring commented Mar 28, 2016

@droe
Copy link
Owner

droe commented Mar 28, 2016

I am starting to wonder whether you are actually some kind of ELIZA specialized on suggesting code improvements. Can you submit at least a single valid pull request to prove that you are actually a human being capable of contributing code?

droe added a commit that referenced this issue Mar 28, 2016
@elfring
Copy link
Author

elfring commented Mar 29, 2016

… some kind of ELIZA …

You can also notice (if you look around a bit more) that I contributed some concrete patches for various free software.
It can occasionally happen that this software will be fixed quicker by other contributors (than me).

Thanks for another small source code improvement.
Unfortunately, there are still more update candidates left over.

@droe
Copy link
Owner

droe commented Mar 29, 2016

thrqueue_enqueue() is a wont-fix, because those functions cannot fail under reasonable conditions. In my eyes, the cost/benefit ratio is not in favour of adding unneeded dead code and complexity there.

@elfring
Copy link
Author

elfring commented Mar 29, 2016

Do you like any reminders for the incomplete error detection (and corresponding exception handling) more from other contributors (or automatic source code analysis tools)?

@droe
Copy link
Owner

droe commented Mar 29, 2016

I'm afraid that I don't understand your question. I always welcome specific and thoughtful bug reports, regardless from whom.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants