From @nwc10

So I'm hacking on the core​:

perl.c​: In function `S_incpush'​:
perl.c​:4435​: warning​: implicit declaration of function `newSVpvf'
perl.c​:4435​: warning​: assignment makes pointer from integer without a cast

But in perlapi.pod​:

=item newSVpvf

Creates a new SV and initializes it with the string formatted like
C<sprintf>.

  SV* newSVpvf(const char* pat, ...)

embed.fnc says

Afpd |SV* |newSVpvf |const char* pat|...

where 'p' is the important flag​:

: p function has a Perl_ prefix

which means that there's no #define to create newSVpvf.

So is perlapi.pod buggy? Should autodoc.pl be changed to generate this entry?

  SV* Perl_newSVpvf(const char* pat, ...)

for the entry without the Perl_ prefix?

Nicholas Clark

From stas@stason.org

Nicholas Clark (via RT) wrote​:

# New Ticket Created by Nicholas Clark
# Please include the string​: [perl #33156]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org​:80/rt3/Ticket/Display.html?id=33156 >

So I'm hacking on the core​:

perl.c​: In function `S_incpush'​:
perl.c​:4435​: warning​: implicit declaration of function `newSVpvf'
perl.c​:4435​: warning​: assignment makes pointer from integer without a cast

But in perlapi.pod​:

=item newSVpvf

Creates a new SV and initializes it with the string formatted like
C<sprintf>.

SV\*    newSVpvf\(const char\* pat\, \.\.\.\)

embed.fnc says

Afpd |SV* |newSVpvf |const char* pat|...

where 'p' is the important flag​:

: p function has a Perl_ prefix

which means that there's no #define to create newSVpvf.

So is perlapi.pod buggy? Should autodoc.pl be changed to generate this entry?

SV\*    Perl\_newSVpvf\(const char\* pat\, \.\.\.\)

for the entry without the Perl_ prefix?

It's broken. Apparently it depends on how your perl is built. We had this
problem when trying to use it in mp2​:
http​://www.mail-archive.com/dev@​perl.apache.org/msg09218.html
in particular​:
http​://www.mail-archive.com/dev@​perl.apache.org/msg09181.html

so we ended up using Perl_ form.

--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http​://stason.org/ mod_perl Guide ---> http​://perl.apache.org
mailto​:stas@​stason.org http​://use.perl.org http​://apacheweek.com
http​://modperlbook.org http​://apache.org http​://ticketmaster.com

The RT System itself - Status changed from 'new' to 'open'

From @jkeenan

On Wed Dec 22 11​:45​:31 2004, nicholas wrote​:

So I'm hacking on the core​:

perl.c​: In function `S_incpush'​:
perl.c​:4435​: warning​: implicit declaration of function `newSVpvf'
perl.c​:4435​: warning​: assignment makes pointer from integer without a
cast

But in perlapi.pod​:

=item newSVpvf

Creates a new SV and initializes it with the string formatted like
C<sprintf>.

SV\*    newSVpvf\(const char\* pat\, \.\.\.\)

embed.fnc says

Afpd |SV* |newSVpvf |const char* pat|...

where 'p' is the important flag​:

: p function has a Perl_ prefix

which means that there's no #define to create newSVpvf.

So is perlapi.pod buggy? Should autodoc.pl be changed to generate this
entry?

SV\*    Perl\_newSVpvf\(const char\* pat\, \.\.\.\)

for the entry without the Perl_ prefix?

Nicholas Clark

Is this still an issue?

From @nwc10

On Sat, May 26, 2012 at 06​:43​:01PM -0700, James E Keenan via RT wrote​:

On Wed Dec 22 11​:45​:31 2004, nicholas wrote​:

So I'm hacking on the core​:

perl.c​: In function `S_incpush'​:
perl.c​:4435​: warning​: implicit declaration of function `newSVpvf'
perl.c​:4435​: warning​: assignment makes pointer from integer without a
cast

But in perlapi.pod​:

=item newSVpvf

Creates a new SV and initializes it with the string formatted like
C<sprintf>.

SV\*    newSVpvf\(const char\* pat\, \.\.\.\)

embed.fnc says

Afpd |SV* |newSVpvf |const char* pat|...

where 'p' is the important flag​:

: p function has a Perl_ prefix

which means that there's no #define to create newSVpvf.

So is perlapi.pod buggy? Should autodoc.pl be changed to generate this
entry?

SV\*    Perl\_newSVpvf\(const char\* pat\, \.\.\.\)

for the entry without the Perl_ prefix?

Nicholas Clark

Is this still an issue?

Yes.

And I think I'm going to also (mostly) agree with myself from 8 years ago as
to what the fix is. Generate this​:

SV* Perl_newSVpvf(aTHX_ const char* pat, ...)

for all functions that have variable argument lists.

Nicholas Clark

From @dcollinsn

Working off of Nick's original report...

In embed.h​:

#ifndef PERL_IMPLICIT_CONTEXT
#define newSVpvf Perl_newSVpvf
#endif

...

#if defined(PERL_IMPLICIT_CONTEXT) && !defined(PERL_NO_SHORT_NAMES)
# define newSVpvf Perl_newSVpvf_nocontext

So it seems that newSVpvf is (almost) always defined (except under PERL_NO_SHORT_NAMES, which seems to be designed not to define it). Is it part of the API now, or should perldoc perlapi still be fixed to call it Perl_newSVpvf?

From @dcollinsn

If we do want perlapi.pod to use the Perl_ prefix...

embed.fnc​:

  Afpda |SV* |newSVpvf |NN const char *const pat|...

: f Function takes a format string. If the function name /strftime/
: then its assumed to take a strftime-style format string as 1st arg;
: otherwise it's assumed to be a printf style format string, varargs
: (hence any entry that would otherwise go in embed.h is suppressed)​:
:
: proto.h​: add __attribute__format__ (or ...null_ok__)
:
: p Function in source code has a Perl_ prefix​:
:
: proto.h​: function is declared as Perl_foo rather than foo
: embed.h​: "#define foo Perl_foo" entries added
:
: o Has no Perl_foo or S_foo compatibility macro​:
:
: embed.h​: suppress "#define foo Perl_foo"
:

Currently autodoc only looks for flags =~ /o/ to add the Perl_ prefix to the function name. If the quote above is to be believed, it should also look for flags =~ /f/ && name !~ /strftime/

But for some reason the f flag never makes it into docout() in autodoc.pl. I'm not sure why we don't just pass in all the flags from embed.fnc, but I modified autodoc.pl to at least pass in and check 'f', and prepend Perl_ if it is present and name !~ /strftime/.

pod/perlapi.pod is not under version control, but comparing before and after gives these changes​:

5844c5844
< char* form(const char* pat, ...)

  char\*   Perl\_form\(const char\* pat\, \.\.\.\)

5965c5965
< SV * mess(const char *pat, ...)

  SV \*    Perl\_mess\(const char \*pat\, \.\.\.\)

6006,6007c6006,6007
< int my_snprintf(char *buffer, const Size_t len,
< const char *format, ...)

  int     Perl\_my\_snprintf\(char \*buffer\, const Size\_t len\,
                           const char \*format\, \.\.\.\)

9679c9679
< SV* newSVpvf(const char *const pat, ...)

  SV\*     Perl\_newSVpvf\(const char \*const pat\, \.\.\.\)

10027,10028c10027,10028
< void sv_catpvf(SV *const sv, const char *const pat,
< ...)

  void    Perl\_sv\_catpvf\(SV \*const sv\,
                         const char \*const pat\, \.\.\.\)

10038,10039c10038,10039
< void sv_catpvf_mg(SV *const sv,
< const char *const pat, ...)

  void    Perl\_sv\_catpvf\_mg\(SV \*const sv\,
                            const char \*const pat\, \.\.\.\)

10887,10888c10887,10888
< void sv_setpvf(SV *const sv, const char *const pat,
< ...)

  void    Perl\_sv\_setpvf\(SV \*const sv\,
                         const char \*const pat\, \.\.\.\)

10898,10899c10898,10899
< void sv_setpvf_mg(SV *const sv,
< const char *const pat, ...)

  void    Perl\_sv\_setpvf\_mg\(SV \*const sv\,
                            const char \*const pat\, \.\.\.\)

13998c13998
< void croak(const char *pat, ...)

  void    Perl\_croak\(const char \*pat\, \.\.\.\)

14045c14045
< OP * die(const char *pat, ...)

  OP \*    Perl\_die\(const char \*pat\, \.\.\.\)

14125c14125
< void warn(const char *pat, ...)

  void    Perl\_warn\(const char \*pat\, \.\.\.\)

Patch to autodoc.pl is attached, if those changes are correct.

From @dcollinsn

From 8f7279b78678d5dc2489277c4d29addd94b774e8 Mon Sep 17 00:00:00 2001
From: Dan Collins <dcollinsn@gmail.com>
Date: Thu, 7 Jul 2016 13:58:49 -0400
Subject: [PATCH] [RT #33156] autodoc: Perl_ prefix for varargs functions

---
 autodoc.pl | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/autodoc.pl b/autodoc.pl
index 161310d..9216608 100644
--- a/autodoc.pl
+++ b/autodoc.pl
@@ -128,6 +128,7 @@ DOC:
 		$embed_where = $docref->{flags} =~ /A/ ? 'api' : 'guts';
 		$embed_may_change = $docref->{flags} =~ /M/;
                 $flags .= 'D' if $docref->{flags} =~ /D/;
+                $flags .= 'f' if $docref->{flags} =~ /f/;
 	    } else {
 		$missing{$name} = $file;
 	    }
@@ -214,7 +215,9 @@ removed without notice.\n\n$docs" if $flags =~ /x/;
 	print $fh "\t$ret\t$name\n\n";
     } else { # full usage
 	my $p            = $flags =~ /o/; # no #define foo Perl_foo
-	my $n            = "Perl_"x$p . $name;
+	my $v            = $flags =~ /f/ &&
+	                   $name !~ /strftime/; # Varargs, Perl_ prefix in doc
+	my $n            = "Perl_"x($p | $v) . $name;
 	my $large_ret    = length $ret > 7;
 	my $indent_size  = 7+8 # nroff: 7 under =head + 8 under =item
 	                  +8+($large_ret ? 1 + length $ret : 8)
-- 
2.8.1

From @khwilliamson

No one responded to this. Is there a reason not to apply this patch in 5.27?
--
Karl Williamson

From @iabyn

I don't understand this issue at all. Plenty of dual-lifed XS code (such
as Encode) use a bare 'newSVpvf' rather than a full 'Perl_newSVpvf' in the
src code. So why does perlapi need fixing to use the long form?

--
This email is confidential, and now that you have read it you are legally
obliged to shoot yourself. Or shoot a lawyer, if you prefer. If you have
received this email in error, place it in its original wrapping and return
for a full refund. By opening this email, you accept that Elvis lives.

From @khwilliamson

I finally took the time to understand this issue better.

Nowadays, the p flag means not what apparently it did at the time of the original ticket, but that the actual function in the source is named Perl_foo. Generally, a macro is generated by the name 'foo' unless the 'o' flag is also specified, or perl is Configured with -DPERL_NO_SHORT_NAMES. Thus there are plenty of calls to plain newSVpvf without a 'Perl_' prefix, and they work. But they would fail under PERL_NO_SHORT_NAMES; and its any function, s not just newSVpvf. It would seem that the fix to the proximal problem would be to change autodoc to add the o flag to every entry when PERL_NO_SHORT_NAMES is defined. That would mean the generated perlapi would be much more configuration dependent than it is now.

But what happens if I do compile without short names. I believe that any module I try to use that used the short names wouldn't compile.

And Devel​::PPPort depends on modules using short names, so that it can potentially redefine the macro for a given short name to do the right thing, rather than the buggy first implementation.

And, concerning name space pollution. Right now, if someone #defines a macro in a perl header file, that macro pollutes the name space of all XS code. Apparently this hasn't been a problem. There is at least a workaround for this, unlike real function calls, as the XS code can #undef the macro and redefine it.

Finally, in the ticket it looks like the example cited by Stas is violating the modern norm by pretending to be PERL_CORE when it isn't. Perhaps that was acceptable back then; I wasn't involved at the time, but nowadays we don't support such things.

--
Karl Williamson

From @khwilliamson

And I looked at it again. And I think this ticket is closable. There is a macro that expands to the proper Perl_ function. And it looks like it existed at the time of the original ticket. And it is currently in use, successfully. If we compile with no short names, not only this one, but every shortened form are forbidden.

I thought that perhaps the macro had changed over time, so that at the time this was written, because of the variable number of args, the macro didn't work. But grepping through the source of lots of different perl versions, it looks like it was always defined correctly to avoid the vararg issue.

So I'll close this in a month-ish if I don't hear anything to the contrary
--
Karl Williamson

No comments in 2.5 months, so closing

Closing, as threatened