Austin Group Defect Tracker

Aardvark Mark IV

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

- Relationships
related to 0000384Closedajosey the stdarg macros should not modify errno 

-  Notes
(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.

- Issue History
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
Powered by Mantis Bugtracker