Bug 168719 - [patch] malloc(3): It seems consensus is to track unpublished standard, so free() need to save/restore errno.
Summary: [patch] malloc(3): It seems consensus is to track unpublished standard, so fr...
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: standards (show other bugs)
Version: 9.0-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Jason Evans
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-06-05 17:40 UTC by Andrey A. Chernov
Modified: 2020-01-21 21:29 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrey A. Chernov freebsd_committer 2012-06-05 17:40:12 UTC
According to the current POSIX standard, errno must be saved across the
free() call, it is also recommendation of the future standard version
(Resolved state): http://austingroupbugs.net/view.php?id=385#c713

"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;
    }
"
"earlier" here is current version. Please read whole problem description
and discussion at this URL too.

When I attempt to commit that recommended "save errno across free()"
way in realpath.c I got enough feedback showing that many members tend
to avoid mass code change needed for current standard strict compliance
and prefer to track unpublished standard instead, where free() must
not modify errno.

-current and -stable free() versions may issue some syscalls which
may modify errno (depends on debug flags, KTR, etc),
so I suggest to directly save and restore errno in the free()
to avoid any possible modifications.

Fix: 

+       serrno = errno;
+
	UTRACE(ptr, 0, 0);
	if (ptr != NULL) {
		assert(malloc_initialized);

		idalloc(ptr);
	}
+       errno = serrno;
 }

 /*--kEl2jvLSY3mvFiM9EyoGOZgtdgZS47KvGGcgk53D6HzyaFpB
Content-Type: text/plain; name="file.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="file.diff"

Index: malloc.c
===================================================================
--- malloc.c    (revision 236135)
+++ malloc.c    (working copy)
@@ -6146,13 +6146,17 @@
 void
 free(void *ptr)
 {
+       int serrno;
Comment 1 Andrey A. Chernov freebsd_committer 2012-06-14 00:54:06 UTC
Responsible Changed
From-To: freebsd-standards->jasone

Reassign to our malloc() maintainer
Comment 2 Eitan Adler freebsd_committer freebsd_triage 2018-05-23 10:27:12 UTC
batch change of PRs untouched in 2018 marked "in progress" back to open.