Anonymous | Login | 2023-12-05 05:50 UTC |
Main | My View | View Issues | Change Log | Docs |
Viewing Issue Simple Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||
ID | Category | Severity | Type | Date Submitted | Last Update | ||
0000385 | [1003.1(2008)/Issue 7] System Interfaces | Objection | Omission | 2011-02-22 21:41 | 2020-12-17 13:33 | ||
Reporter | eblake | View Status | public | ||||
Assigned To | ajosey | ||||||
Priority | normal | Resolution | Accepted As Marked | ||||
Status | Applied | ||||||
Name | Eric Blake | ||||||
Organization | Red Hat | ||||||
User Reference | ebb.free | ||||||
Section | free | ||||||
Page Number | 915 | ||||||
Line Number | 30583 | ||||||
Interp Status | --- | ||||||
Final Accepted Text | Note: 0000713 | ||||||
Summary | 0000385: free should not change errno on success | ||||||
Description |
Line 25639 [XSH errno] is explicit that "The setting of errno after a successful call to a function is unspecified unless the description of that function specifies that errno shall not be modified." However, free() does not mention its interaction with errno, which renders common code like this undefined, all because the successful free() invocation occurs between the point of the failure and the actual error reporting: ret = write(fd, buf, buflen); if (ret < 0) { free(buf); return errno; } free() does not have a return value, and does not have any errors required by the standard. However, it is a useful extension to permit applications to leave errno unchanged on success, and set to a value (such as EINVAL or EFAULT) when passed a bogus pointer. With the current standard, applications that perform cleanup on error paths must currently save and restore errno around calls to free(), but this can get expensive in run-time cost where errno evaluates to a function call (such as (*__errno())). If the free() implementation were to guarantee that all valid arguments leave errno unchanged, then applications could be made slightly more efficient. |
||||||
Desired Action |
At line 30583 [XSH free DESCRIPTION], add a paragraph with CX shading: The free() function shall not modify errno if ptr is a null pointer or a pointer previously returned as if by malloc() and not yet deallocated. At line 30593 [RATIONALE], change "None." to: This standard leaves behavior undefined if free() is passed an invalid argument; however, since free() must succeed on a valid pointer and must not modify errno in the process, an implementation may provide an extension where failure to free an invalid pointer can be detected by observing a modification to errno. |
||||||
Tags | issue8 | ||||||
Attached Files | |||||||
|
![]() |
||||||
|
![]() |
|
(0000676) geoffclare (manager) 2011-02-23 11:46 |
If we make this change for free() we should do the same for the related functions freeaddrinfo(), freelocale(), globfree(), if_freenameindex(), regfree(), and wordfree(). |
(0000712) eblake (manager) 2011-03-17 19:14 |
Additional functions that return void but perform cleanup, and could feasibly be extended to detect error conditions, and therefore might be worth adjusting to describe their effects on errno: closelog() dbm_close() endgrent()/endhostent()/endprotoent()/endpwent()/endservent()/endutxent() flockfile()/funlockfile() However, I will focus on just the *free* interfaces in this bug, and we can open separate bugs for the others as desired. |
(0000713) eblake (manager) 2011-03-17 21:30 edited on: 2011-03-24 15:35 |
At line 30583 [XSH free DESCRIPTION], add a paragraph with CX shading: The free() function shall not modify errno if ptr is a null pointer or a pointer previously returned as if by malloc() and not yet deallocated. At line 30591 [APPLICATION USAGE], add a new paragraph: Because the free() function does not modify errno for valid pointers, it is safe to use it in cleanup code without corrupting earlier errors, such as in this example code: // buf was obtained by malloc(buflen) ret = write(fd, buf, buflen); if (ret < 0) { free(buf); return ret; } However, earlier versions of this standard did not require this, and the same example had to be written as: // buf was obtained by malloc(buflen) ret = write(fd, buf, buflen); if (ret < 0) { int save = errno; free(buf); errno = save; return ret; } At line 30620 [freeaddrinfo DESCRIPTION], add a sentence: The freeaddrinfo() function shall not modify errno if ai is a sublist previously returned by getaddrinfo() and not yet freed. At line 30834 [freelocale DESCRIPTION], add a sentence: The freelocale() function shall not modify errno if locobj is a locale object previously returned by newlocale() or duplocale() and not yet released by freelocale() or newlocale(). At line 36254 [glob DESCRIPTION], add a sentence: The globfree() function shall not modify errno if pglob was previously used by glob() and not yet freed. At line 36873 [if_freenameindex DESCRIPTION], add a sentence: The if_freenameindex() function shall not modify errno if ptr was previously returned by if_nameindex() and not yet freed. At line 56540 [regcomp DESCRIPTION], add a sentence: The regfree() function shall not modify errno if preg was previously returned by regcomp() and not yet freed. At line 70965 [wordexp DESCRIPTION], add a sentence: The wordfree() function shall not modify errno if pwordexp was previously modified by wordexp() and not yet freed. |
(0000714) Konrad_Schwarz (reporter) 2011-03-18 10:04 |
I fail to see how an application can benefit from observing a failure in free() (e.g., via errno). Such a failure indicates some flaw in the program; its state has been corrupted, how can it continue safely? I would argue the most sensible thing a library function can do on detecting such state inconsistencies is to bring this to the attention of a human operator/programmer as quickly and productively as possible, e.g., by producing a core dump useful in post-mortem analysis. Adding the proposed text to the rationale may mislead some implementors to conclude that reporting such state inconsistencies via errno rather than a call to abort() is a good idea. This requires applications to change: they must start checking their calls to free() et al. via awkward errno saving/testing/restoring schemes. This is unlikely to happen. The end result is that such errors will slip through unnoticed, potentially causing havoc at a much later stage in the program, when it is much more difficult to pin-point the root cause of the problem. Thus, I would remove the wording suggested for the [RATIONALE] sections. |
(0000719) geoffclare (manager) 2011-03-24 15:41 |
(Response to Note: 0000714) The changes in Note: 0000713 have been updated to remove the RATIONALE sections as suggested. |
![]() |
|||
Date Modified | Username | Field | Change |
2011-02-22 21:41 | eblake | New Issue | |
2011-02-22 21:41 | eblake | Status | New => Under Review |
2011-02-22 21:41 | eblake | Assigned To | => ajosey |
2011-02-22 21:41 | eblake | Name | => Eric Blake |
2011-02-22 21:41 | eblake | Organization | => Red Hat |
2011-02-22 21:41 | eblake | User Reference | => ebb.free |
2011-02-22 21:41 | eblake | Section | => free |
2011-02-22 21:41 | eblake | Page Number | => 915 |
2011-02-22 21:41 | eblake | Line Number | => 30583 |
2011-02-22 21:41 | eblake | Interp Status | => --- |
2011-02-23 11:46 | geoffclare | Note Added: 0000676 | |
2011-03-17 19:14 | eblake | Note Added: 0000712 | |
2011-03-17 21:30 | eblake | Note Added: 0000713 | |
2011-03-18 10:04 | Konrad_Schwarz | Note Added: 0000714 | |
2011-03-18 17:51 | eblake | Note Edited: 0000713 | |
2011-03-24 15:35 | eblake | Note Edited: 0000713 | |
2011-03-24 15:39 | geoffclare | Final Accepted Text | => Note: 0000713 |
2011-03-24 15:39 | geoffclare | Status | Under Review => Resolved |
2011-03-24 15:39 | geoffclare | Resolution | Open => Accepted As Marked |
2011-03-24 15:40 | geoffclare | Tag Attached: issue8 | |
2011-03-24 15:41 | geoffclare | Note Added: 0000719 | |
2012-05-11 18:03 | eadler | Issue Monitored: eadler | |
2016-07-28 15:06 | eblake | Relationship added | related to 0000384 |
2020-02-21 16:28 | geoffclare | Status | Resolved => Applied |
2020-12-17 13:33 | eblake | Description Updated |
Mantis 1.1.6[^] Copyright © 2000 - 2008 Mantis Group |