After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 634154 - Check return codes everywhere
Check return codes everywhere
Status: RESOLVED WONTFIX
Product: gparted
Classification: Other
Component: application
0.7.0
Other All
: Normal normal
: ---
Assigned To: gparted maintainers alias
gparted maintainers alias
Depends on:
Blocks:
 
 
Reported: 2010-11-06 10:12 UTC by Markus Elfring
Modified: 2015-10-21 17:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
update suggestion (3.16 KB, patch)
2010-11-07 14:45 UTC, Markus Elfring
none Details | Review

Description Markus Elfring 2010-11-06 10:12:20 UTC
I have noticed that the return value from the function "pthread_create" is ignored in your member function "Dialog_Progress::on_signal_show" so far.
http://git.gnome.org/browse/gparted/tree/src/Dialog_Progress.cc?id=d040e4e5847905f58207e093917b6c3d36e641d7#n190

Would you like to complete error detection and corresponding exception handling?

Are more places in the source files affected in a similar way?
Comment 1 Markus Elfring 2010-11-07 14:45:49 UTC
Created attachment 173990 [details] [review]
update suggestion

Should C++ exceptions be caught by the implementation of the function "main"?

Would you like to integrate the appended adjustment into your source code repository?
Comment 2 Curtis Gedak 2011-10-16 17:59:38 UTC
Markus, this is a good example of a problem you discovered and also provided a patch.

However, this is low on my priority list.

One question that comes to my mind is:
  What impact does this change have on producing a stack trace of a program crash?
Comment 3 Markus Elfring 2011-10-17 17:51:52 UTC
(In reply to comment #2)
> What impact does this change have on producing a stack trace of a program crash?

My attachment #173990 [details] affects more the (in)dependence from C++ compiler implementations. Are you concerned about "exception fall through" in the C function "main"?
Comment 4 Curtis Gedak 2011-10-17 19:21:45 UTC
(In reply to comment #3)
> Are you concerned about "exception fall through" in the C
> function "main"?

I am more concerned with error and return code handling from the libparted library.  Some libparted errors seek a response from the user.  Currently GParted does not permit a user to respond.  Better handling of libparted errors might enable us to fix the following bug report:

Bug #566935 - Unable to expand Areca RAID partition from 2.6TB to 3.4TB
Comment 5 Curtis Gedak 2011-10-17 19:24:06 UTC
(In reply to comment #3)
> Are you concerned about "exception fall through" in the C
> function "main"?

I am concerned about how program crashes can be traced and solved.  Since I am not an expert in this area I am reluctant to make changes that might adversely impact the ability to troubleshoot program crashes.
Comment 6 Markus Elfring 2011-10-18 15:29:59 UTC
(In reply to comment #5)
> I am concerned about how program crashes can be traced and solved.  Since I am
> not an expert in this area I am reluctant to make changes that might adversely
> impact the ability to troubleshoot program crashes.

It can be that a specific version range of a C++ compiler implementation provides more detailed error reporting than the suggested approach (because "main()" is wrapped by a special function with an included catch-all clause). But how do you generally know if this potentially "verbose alternative" is really better for the safe handling of an uncaught exception?

Can any informations from a previous discussion on a topic like "Catch exceptions in main()" help in the clarification for the open issue here?
https://sourceforge.net/apps/trac/cppcheck/ticket/2288

How do you think about the article "Quality Matters #6: Exceptions for Practically-Unrecoverable Conditions" by Matthew Wilson?
http://accu.org/index.php/journals/1706
Comment 7 Phillip Susi 2013-02-28 20:22:57 UTC
Hi Curtis, I think this one can be closed as soon as you finish testing and merge my patch set, since the pthread_create call has been removed.
Comment 8 Curtis Gedak 2013-02-28 22:33:57 UTC
Hi Phillip.  With this report I think the purpose of the patch in comment #1 is to set up an error handler to catch all unhandled errors.  With this in mind, removing the pthread_create call should not invalidate this report.
Comment 9 Phillip Susi 2013-02-28 23:59:21 UTC
The bit about exception catching seems a bit redundant since gnome does this automatically.
Comment 10 Curtis Gedak 2013-03-01 16:27:19 UTC
Thanks Phillip for the tip about gnome (I assume gtkmm) doing this automatically.

Since the GParted code contains both C and C++ code, it appears that using C++ exceptions might be a bad idea, at least according to the FAQ.

https://live.gnome.org/gtkmm/FAQ

----- BEGIN QUOTE -----
Can I use C++ exceptions with gtkmm?

Yes, it is possible but it is not a very good idea. The official answer is that, since plain C doesn't know what a C++ exception is, you can use exceptions in your gtkmm code as long as there are no C functions in your call stack between the thrower and the catcher. This means that you have to catch your exception locally.

You will be warned at runtime about uncaught exceptions, and you can specify a different handler for uncaught exceptions. Some gtkmm methods do even use exceptions to report errors. The exceptions types that might be thrown are listed in the reference documentation of these methods. 
----- END QUOTE -----
Comment 11 Phillip Susi 2013-03-01 18:49:56 UTC
Fortunately we aren't really using exceptions, so we should be good.  So that leaves us with nothing here to fix, so I think this report can be closed.
Comment 12 Markus Elfring 2013-03-01 19:33:51 UTC
(In reply to comment #11)

Would you like to handle the exception "std::bad_alloc" (and other unexpected things) eventually?
Comment 13 Phillip Susi 2013-03-01 20:16:24 UTC
Handle it how?  If you run out of memory there isn't really anything you can do besides die.
Comment 14 Markus Elfring 2013-03-01 21:11:23 UTC
(In reply to comment #13)

Do you moan eventually before the software "dies" because of an exceptional situation?   ;-)
Comment 15 Phillip Susi 2013-03-01 21:27:02 UTC
I'm afraid I don't understand you.
Comment 16 Markus Elfring 2013-03-01 21:40:44 UTC
(In reply to comment #15)

My attachment #173990 [details] shows a possibility to become independent from implementation-defined behaviour of C++ compilers, doesn't it?
Comment 17 Phillip Susi 2013-03-01 22:01:36 UTC
What implementation defined behavior?
Comment 19 Phillip Susi 2013-03-02 02:02:18 UTC
I still have no idea what you are trying to say because you seem to be making veiled hints.  What, exactly, are you proposing?
Comment 20 Markus Elfring 2013-03-02 11:30:26 UTC
(In reply to comment #19)

1. Please do not ignore the return value from "pthread_create()" in your member function "Dialog_Progress::on_signal_show".
   http://git.gnome.org/browse/gparted/tree/src/Dialog_Progress.cc?id=7c5b5edaef865652225c420946595518419ea614#n193

2. I suggest to apply the advices by Matthew Wilson for a "production-quality main()". Do you find the article "Quality Matters #6: Exceptions for Practically-Unrecoverable Conditions" useful also for a partition management tool?
   http://accu.org/index.php/journals/1706
Comment 21 Phillip Susi 2013-03-02 17:55:36 UTC
As I said before, #1 is taken care of in a patch set that is about to be merged because pthread_create() will no longer be called.  For the rest, you are still being too vague and hand wavy to understand.  What specific error condition needs handled?
Comment 22 Markus Elfring 2013-03-02 18:08:59 UTC
(In reply to comment #21)

1. May I see your patch also? (Where can it be reviewed?)

2. Did you read any of the linked information sources?
Comment 23 Phillip Susi 2013-03-02 23:04:03 UTC
Bug 685740 is tracking it and you can find it in the git repository under the psusi/refactor branch.
Comment 24 Curtis Gedak 2015-10-21 17:17:58 UTC
Closing this report as RESOLVED - WONTFIX because we do not plan to add C++ exception handling to GParted in part due to the reason listed in comment #10.