[Bug 14788] New: Check return codes everywhere
http://bugs.winehq.org/show_bug.cgi?id=14788 Summary: Check return codes everywhere Product: Wine Version: CVS/GIT Platform: All OS/Version: All Status: UNCONFIRMED Severity: normal Priority: P2 Component: -unknown AssignedTo: wine-bugs(a)winehq.org ReportedBy: Markus.Elfring(a)web.de Some source files contain unchecked function calls. Examples: - malloc => main http://source.winehq.org/git/wine.git/?a=blob;f=tools/fnt2fon.c;h=aa7d72b84b... => build_new_path http://source.winehq.org/git/wine.git/?a=blob;f=loader/freebsd.c;h=0d0eb5767... => pdb_jg_read http://source.winehq.org/git/wine.git/?a=blob;f=tools/winedump/pdb.c;h=1d9fc... - strdup => build http://source.winehq.org/git/wine.git/?a=blob;f=tools/winegcc/winegcc.c;h=11... - fprintf => writeHeader http://source.winehq.org/git/wine.git/?a=blob;f=dlls/wineps.drv/afm2c.c;h=3f... - fputs => REGPROC_export_string http://source.winehq.org/git/wine.git?a=blob;f=programs/regedit/regproc.c;h=... - pthread_mutex_lock, pthread_mutex_unlock => __pthread_atfork http://source.winehq.org/git/wine.git/?a=blob;f=loader/kthread.c;h=cc51cc060... Would you like to add more error handling for corresponding return values? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Vitaliy Margolen <vitaliy(a)kievinfo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement --- Comment #1 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2008-08-07 12:07:33 --- Send patches to wine-patches mailing list. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #2 from Markus Elfring <Markus.Elfring(a)web.de> 2008-08-07 13:01:55 --- Would you like to detect every error situation as early as possible? I am unsure which approaches will be acceptable in your software to complete error detection and handling. - How do you think about the reaction "exit(errno)" or "abort()"? - Would you like to reduce the efforts for error code checking by an exception class hierarchy? http://dietmar-kuehl.de/mirror/c++-faq/exceptions.html#faq-17.1 http://cexcept.sourceforge.net/ -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #3 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2008-08-07 15:22:24 --- Move this discussion to wine-devel. Some stuff you pointed out is valid, some - irrelevant. It makes no sense checking all the output in secondary utilities. It will make their code more complicated for no good reason. Also it seems you totally don't understand what Wine is, and how much of c++ it has... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Juan Lang <juan_lang(a)yahoo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |INVALID --- Comment #4 from Juan Lang <juan_lang(a)yahoo.com> 2008-08-07 21:12:47 --- I'm marking this invalid. Several times memory checking, leaking, etc. within the utilities used to build wine have been brought up, and each time Alexandre says it's not worth the effort. We don't have many "tracker" bugs here, so discussing possible enhancements throughout the codebase doesn't seem like a good use of bugzilla. Besides this, most of the issues you brought up are not recoverable errors anyway, so there appears to be no benefit of checking the return values in most of the instances you point out. If you'd like to fix a particular instance, either send a patch to wine-patches, or discuss it on wine-devel. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #5 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-08-08 03:54:42 --- Closing. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #6 from Markus Elfring <Markus.Elfring(a)web.de> 2008-08-08 06:21:57 --- I know that it is hard to achieve consensus on details for approaches to do "the right thing". I try to get acceptance for potential updates before I would fiddle with source code. Would you like to look at opinions from the discussion "PTHREAD_MUTEX_INITIALIZER"? http://groups.google.de/group/comp.programming.threads/browse_frm/thread/1d3... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #7 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-08-08 07:28:14 --- As Vitaliy said feel free to send the patches to wine-patches, and start the discussion at wine-devel. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #8 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-13 03:43:33 --- Created an attachment (id=16068) --> (http://bugs.winehq.org/attachment.cgi?id=16068) update suggestion Would you like to integrate any of the proposed changes for improved software stability? How do you think about to look for any remaining issues by static source code analysis? http://en.wikipedia.org/wiki/List_of_tools_for_static_code_analysis#C -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Austin English <austinenglish(a)gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |patch -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Vitaliy Margolen <vitaliy(a)kievinfo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16068|text/plain |application/octet-stream mime type| | Attachment #16068|1 |0 is patch| | -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #9 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2008-09-13 11:40:45 --- I appreciate your effort, however most changes are invalid. You can't just "exit(errno)" when fclose() failed! This is as harmless as it gets. Yet you just crash the whole application because of it. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #10 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 02:21:11 --- Which reaction do you like more for failed (f)close function calls? http://en.wikipedia.org/wiki/Crash-only_software -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #11 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 02:31:37 --- (In reply to comment #10)
Which reaction do you like more for failed (f)close function calls?
No, handling fclose() failures is absolutely useless. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #12 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 02:51:02 --- I have got the opinion that such destruction or resource release failures should not be ignored. Will it make sense to adjust the reactions by distinguishing the error indication for file descriptors that became invalid and interruption of file operations? http://opengroup.org/onlinepubs/009695399/functions/close.html http://opengroup.org/onlinepubs/009695399/functions/fclose.html -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #13 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 03:01:56 --- (In reply to comment #12)
I have got the opinion that such destruction or resource release failures should not be ignored. Will it make sense to adjust the reactions by distinguishing the error indication for file descriptors that became invalid and interruption of file operations? http://opengroup.org/onlinepubs/009695399/functions/close.html http://opengroup.org/onlinepubs/009695399/functions/fclose.html
These worries are theoretical. If you find out that some resource (you are relying upon) abruptly becomes invalid that would mean a bug in the underlying OS. In any case what would handling such a case buy us? Absolutely nothing. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #14 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 03:04:17 --- Besides, have you looked at the example code provided in the links you posted? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #15 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 03:19:31 --- Yes - I know it is common to be "sloppy" with error checking in such short demonstration code. I prefer to detect every error situation in the source code as soon as possible to improve software robustness and correctness. Would you like to care for (unexpected) memory corruption? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #16 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 03:29:31 --- (In reply to comment #15)
Would you like to care for (unexpected) memory corruption?
If that's again a theoretical speculation - no. If you know an exact problem - open a bug report. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #17 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 03:50:56 --- Memory corruption is not only a theoretical issue. http://en.wikipedia.org/wiki/Buffer_overflow http://en.wikipedia.org/wiki/ECC_memory#Error-correcting_memory I recommend to achieve complete error detection. I do not like ignorance of (unpleasant) return values. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|patch | OS/Version|All |other Platform|All |Other Version|CVS/GIT |unspecified --- Comment #18 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 04:01:57 --- (In reply to comment #17)
Memory corruption is not only a theoretical issue. http://en.wikipedia.org/wiki/Buffer_overflow http://en.wikipedia.org/wiki/ECC_memory#Error-correcting_memory I recommend to achieve complete error detection.
Looks like you don't have any problems in Wine to point out.
I do not like ignorance of (unpleasant) return values.
As was said several times already not all such cases need an action, at least so far you haven't demonstrated anything useful to worry about. I'd prefer to stop debates which have nothing to do with real bugs in Wine. Again, If you know an exact problem in Wine code - open a bug report, stop speculating about theoretical problems. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Markus Elfring <Markus.Elfring(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- OS/Version|other |All Platform|Other |All Version|unspecified |CVS/GIT --- Comment #19 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 04:18:11 --- Would you like to consider any updates for remaining unchecked function calls? - malloc, strdup - (f)printf, fputs, fputc, ftruncate, unlink - pthread_... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- OS/Version|All |other Platform|All |Other --- Comment #20 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-14 04:30:36 --- Please stop posting i this bug, it's invalid and closed. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Dmitry Timoshkov <dmitry(a)codeweavers.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|CVS/GIT |unspecified -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #21 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-14 09:50:21 --- How do you think about to identify update candidates by compiler-specific extensions like the tag "__attribute__ ((__warn_unused_result__))"? http://gcc.gnu.org/onlinedocs/gcc-4.3.2/gcc/Function-Attributes.html Use cases: (f)read, (f)write, ftell, lseek ... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #22 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2008-09-14 10:59:18 --- (In reply to comment #19)
Would you like to consider any updates for remaining unchecked function calls? - malloc, strdup - (f)printf, fputs, fputc, ftruncate, unlink - pthread_...
As far as malloc, strdup - they should not be used in Wine itself: http://wiki.winehq.org/ReplaceMalloc For other function some places might benefit from proper error checking for these functions. However as I indicate before, each error have to be handled properly not just "exit(errno);". Of course this is relevant to Wine itself and it's tools. Adding these checks to tests is counterproductive. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #23 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-15 08:13:46 --- (In reply to comment #22)
For other function some places might benefit from proper error checking for these functions. However as I indicate before, each error have to be handled properly not just "exit(errno);".
Which function should be called instead? -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #24 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2008-09-15 08:47:07 --- If you reread the comments you will find the answers. Please stop posting in this bug, it's invalid and closed. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #25 from Markus Elfring <Markus.Elfring(a)web.de> 2008-09-19 12:14:18 --- I guess that a well-known analysis tool will remind interested software developers about related open issues by detailed reports. http://scan.coverity.com/rung1.html => http://scan2.coverity.com:7479/ -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Markus Elfring <Markus.Elfring(a)web.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|CLOSED |UNCONFIRMED Resolution|INVALID | --- Comment #26 from Markus Elfring <Markus.Elfring(a)web.de> 2009-03-24 03:57:13 --- Did the desire to look at a topic like "CWE-389: Error Conditions, Return Values, Status Codes" increase after an update for the issue "Fix a memory leak (Coverity 904)" was applied? http://cwe.mitre.org/data/definitions/389.html Will more function implementations be completed for this aspect? Example: WCMD_strdupW http://source.winehq.org/git/wine.git/?a=blob;f=programs/cmd/wcmdmain.c;h=c5... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Vitaliy Margolen <vitaliy(a)kievinfo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|UNCONFIRMED |RESOLVED Resolution| |INVALID --- Comment #27 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2009-03-24 10:21:16 --- Why did you reopened the bug? If you want to fix the return code check - send patches. DO NOT REOPEN. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 Vitaliy Margolen <vitaliy(a)kievinfo.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED --- Comment #28 from Vitaliy Margolen <vitaliy(a)kievinfo.com> 2009-03-24 10:21:32 --- Closing -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #29 from Markus Elfring <Markus.Elfring(a)web.de> 2009-03-24 13:22:43 --- (In reply to comment #27)
[...] - send patches.
My attachment #16068 contains suggestions for software changes. There are still more ways to fix open issues like "CWE-690: Unchecked Return Value to NULL Pointer Dereference" (and "CWE-252: Unchecked Return Value" in general). Are there any chances to apply usual advices by tools from the software area "aspect-oriented programming"? http://research.msrg.utoronto.ca/ACC/Tutorial#A_Reusable_Aspect_for_Memory_A... -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=14788 --- Comment #30 from Dmitry Timoshkov <dmitry(a)codeweavers.com> 2009-03-25 05:13:22 --- This bug is INVALID, which means that there is no bug in Wine as implied here, so no any further action required. -- Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email Do not reply to this email, post in Bugzilla using the above URL to reply. ------- You are receiving this mail because: ------- You are watching all bug changes.
participants (1)
-
wine-bugs＠winehq.org