Should we rename os:: functions that are named like standard C- or Posix-functions?

[Thomas Stüfe]

On Sat, Jul 2, 2022 at 5:22 PM Kim Barrett <kim.barrett at oracle.com> wrote:

> > On Jun 30, 2022, at 1:21 AM, David Holmes <david.holmes at oracle.com>
> wrote:
> >
> > Hi Thomas,
> >
> > On 30/06/2022 2:57 pm, Thomas Stüfe wrote:
> >> Hi,
> >> several functions in the os:: name scope are deliberately named like
> the official counterparts they replace:
> >> os::malloc, os::free, os::strdup, os::realloc, os::recv, os::send,
> os::connect, os::signal...
> >> There may be more. Some of them argument-match their counterparts (e.g.
> os::free), while others don't.
> >> Since the os:: variants can be called inside the os:: namespace with
> omitting the leading os::, name confusions are possible. "free(p)" means
> something different in global scope or inside an os:: function.
> >> This can lead to problems that are difficult to find, e.g., mismatched
> (os::)malloc->(os::)free with the potential to corrupt the C-heap:
> >> […] So I wonder if we should do that. Rename os::<function> to
> something like os::<prefix><function>. And what the prefix or suffix would
> be.
> >
> > It annoys me that we have to do such things. It would have made more
> sense for the standard C library routines to have a prefix that marked them
> as reserved identifiers rather than polluting the global namespace the way
> they did. But no one thinks of these things initially and by the time it is
> standardised it is too late to make such changes. :(
> >
> > I'm not sure this is a problem we have to address, but if we choose to
> then I think we should try to make a general improvement to the way os is
> used.
> >
> > Maybe, as I think has been suggested before, we can move these out of
> the os class as they are not really about the os but the C library, and
> then any renaming that includes a prefix may not look so bad?
> >
> > Maybe lib::C_free(), lib::C_malloc() etc?
>
> A reminder that JDK-8214976 allows us to "poison" a function, other than in
> explicitly marked places. When I introduced that feature I only marked a
> small
> number of functions that were "easy". There are many other functions that
> seem
> like good candidates, but had more fannout than I wanted in that change.
> For
> example, we could mark ::malloc, ::calloc, ::free, &etc as normally
> forbidden.
>
>
I really like this, in addition to the name change. Note however that we
may still need to expose the raw functions to hotspot code for outlier
cases. Something like "os::raw_malloc()" and "os::raw_free()" for "when you
really really mean it".

Incidentally, in our proprietary VM we have a propietary C-heap tracing,
invented before NMT existed. That one covers the whole JDK, not only the
hotspot, and didi not use malloc headers. When we did this, we had a fun
time hunting down all the system APIs the JDK uses that return C-heap and
thus require the caller to raw-free() it. I remember being surprised at the
number. This was across the whole JDK though, which uses more system APIs
than hotspot. Still, with that in mind, we may need at least an
os::raw_free().
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-dev/attachments/20220702/0a6e7d50/attachment-0001.htm>