perl5 icon indicating copy to clipboard operation
perl5 copied to clipboard

Mark do_open(), do_openn() as internal, not public

Open khwilliamson opened this issue 7 months ago • 8 comments

These undocumented functions have exactly 0 uses on CPAN.

do_open() is mentioned in 7 distributions. In all cases it is to #undef the macro name because it was polluting the distro's name space which has its own function named do_open().

  • This set of changes does not require a perldelta entry.

khwilliamson avatar May 24 '25 14:05 khwilliamson

do_open() is used by the standard typemap, some CPAN modules use do_openn() File-Map, mod_perl, Linux-CDROM though I don't know how current any of those are.

tonycoz avatar May 24 '25 15:05 tonycoz

cpan grep shows no uses of do_openn()

https://grep.metacpan.org/search?q=do_openn&qft=&qd=&qifl=ppport.h

And yet it does get called, as you said, in File::Map https://metacpan.org/release/LEONT/File-Map-0.71/source/lib/File/Map.xs

Am I doing something wrong, or is metacpan search buggy?

khwilliamson avatar May 24 '25 15:05 khwilliamson

Am I doing something wrong, or is metacpan search buggy?

If you select *.xs, *.c as the file extensions, four dists will show up: https://grep.metacpan.org/search?q=do_openn&qft=.xs%2C+.c&qd=&qifl=

Leont avatar May 24 '25 16:05 Leont

That sounds like a bug. I want to do an unrestricted search, and yet it doesn't find it.

khwilliamson avatar May 24 '25 17:05 khwilliamson

I have a local (bare) clone of https://github.com/metacpan/metacpan-cpan-extracted.git and I git grep that instead of using grep.metacpan.org (it's a lot faster, but 4.4GB disk space).

tonycoz avatar May 24 '25 23:05 tonycoz

That sounds like a bug. I want to do an unrestricted search, and yet it doesn't find it.

I can't reproduce it anymore, but I did see it right when you posted. I guess it was just glitching or something? I've seen some other weird results today as well.

Leont avatar May 24 '25 23:05 Leont

I have been getting randomly wrong results with the search, so will open a ticket against that.

khwilliamson avatar May 27 '25 13:05 khwilliamson

It turns out that do_open is marked as binary-compatibility only, so, presuming that is correct, there is no need to mark that as internal.

And do_open6 and 'do_open_raw` are marked as experimental. Should that be removed from one or both of them?

khwilliamson avatar May 27 '25 13:05 khwilliamson

And do_open6 and 'do_open_raw` are marked as experimental. Should that be removed from one or both of them?

They aren't API, so it doesn't matter if they're experimental.

Maybe regen should complain if an unexported (or hidden) function is experimental, I don't think it adds any meaning - we're free to change unexported functions anyway.

The other side is maybe they should be API, they'd be less confusing to use than do_openn().

do_open_raw() last changed it's signature in 2017 and do_open6() in 2014, so if they are made API I think they could be non-experimental.

tonycoz avatar Jun 20 '25 00:06 tonycoz

These undocumented functions have exactly 0 uses on CPAN.

do_openn:

https://metacpan.org/release/LEONT/File-Map-0.71/source/lib/File/Map.xs#L457

https://metacpan.org/release/VPARSEVAL/Linux-CDROM-0.02/source/CDROM.xs#L201

https://metacpan.org/release/GFUJI/Ruby-0.07/source/perlio.c#L370

(2 of these are ancient)

do_open: (not dists that just #undef do_open)

https://metacpan.org/release/MLEHMANN/IO-AIO-4.81/source/AIO.xs#L567

https://metacpan.org/release/TODDR/IO-1.55/source/IO.xs#L244 (we maintain this, but it's still on cpan)

(there's more)

several modules use it in their typemap, and it is used in the standard typemap

https://github.com/Perl/perl5/blob/148136c313ee439bdf7c9faed3dbbb772c04a925/lib/ExtUtils/typemap#L425-L477

do_open() is mentioned in 7 distributions. In all cases it is to #undef the macro name because it was polluting the distro's name space which has its own function named do_open().

SWIG adds these to avoid conflicts with C++ code from what I can see.

Others I looked at are specifically C++ XS modules, which undef them for the reasons from #23459.

tonycoz avatar Jul 29 '25 00:07 tonycoz