Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perl 5.5.660 and Sys::Syslog.pm possible minor bug (fwd) #1266

Closed
p5pRT opened this issue Mar 4, 2000 · 5 comments
Closed

Perl 5.5.660 and Sys::Syslog.pm possible minor bug (fwd) #1266

p5pRT opened this issue Mar 4, 2000 · 5 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Mar 4, 2000

Migrated from rt.perl.org#2262 (status was 'resolved')

Searchable as RT2262$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 4, 2000

From tom@compton.nu

---------- Begin forwarded message ----------
Date​: 2 Mar 2000 01​:51​:12 -0000
From​: jon.wright@​awtpl.com.au (Jon Wright)
To​: tom@​compton.nu
Subject​: Perl 5.5.660 and Sys​::Syslog.pm possible minor bug

Hi Tom,

I have been building Perl 5.5.660 on Solaris 7 (32/64) and I have a
question about Sys​::Syslog (which mentions you as the author of the
xs backend).

Basically I am getting errors of the form​:
  Argument "LOG_INFO" isn't numeric in subroutine entry
  at /pkgs/depot/perl5_5660/lib/5.5.660/Sys/Syslog.pm line 126

The line in question is part of the AUTOLOAD code​:
  my $val = constant($constname, @​_ ? $_[0] : 0);

When I check out the xs file in the source directory, it appears that
constant() takes two arguments, the second one is an 'int'. In this
case the value of $_[0] is 'LOG_INFO' which is not 'int'.

Checking further (and I must admit I am not a wiz at autoload/xs) it
seems that this second argument (called arg) is passed all over the
place but it doesn't appear to be actually used.

The perl constant(char,int) calls a C constant(char,int,int) which calls
more constant* routines including the mysterious 'arg' but nothing appears
to reference arg other than to pass it to another subroutine.

I have tentatively replaced the code in question with​:
  my $val = constant($constname, @​_ ? 1 : 0);
and this appears to have no ill effects. I am sorry I cannot test on more
platforms but if there is anything else I can do, please let me know.

Jon Wright
Unix Admin
Sydney Water

----------- End forwarded message -----------

--
Tom Hughes (tom@​compton.nu)
http​://www.compton.nu/
...Law stands mute in the midst of arms.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 4, 2000

From [Unknown Contact. See original ticket]

Jon Wright <jon.wright@​awtpl.com.au> wrote​:

When I check out the xs file in the source directory, it appears that
constant() takes two arguments, the second one is an 'int'. In this
case the value of $_[0] is 'LOG_INFO' which is not 'int'.

Checking further (and I must admit I am not a wiz at autoload/xs) it
seems that this second argument (called arg) is passed all over the
place but it doesn't appear to be actually used.

This code in the Sys​::Syslog XS backend is what h2xs generated
for me - does anyone know what the point of the second argument
to the constant() routine is given that it doesn't seem to be
used and it seems to be passed based on the first argument to
the AUTOLOAD subroutine which, as far as I know, doesn't take
any arguments...

Note that I can't actually reproduce the described problem at the
moment.

Tom

--
Tom Hughes (tom@​compton.nu)
http​://www.compton.nu/
...A fair exterior is a silent recommendation.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 4, 2000

From [Unknown Contact. See original ticket]

Tom Hughes writes​:

When I check out the xs file in the source directory, it appears that
constant() takes two arguments, the second one is an 'int'. In this
case the value of $_[0] is 'LOG_INFO' which is not 'int'.

Checking further (and I must admit I am not a wiz at autoload/xs) it
seems that this second argument (called arg) is passed all over the
place but it doesn't appear to be actually used.

This code in the Sys​::Syslog XS backend is what h2xs generated
for me - does anyone know what the point of the second argument
to the constant() routine is given that it doesn't seem to be
used and it seems to be passed based on the first argument to
the AUTOLOAD subroutine which, as far as I know, doesn't take
any arguments...

I think the original intent was to support macros with one argument.

Well, apparently the sniffer of h2xs filters out such macros. So
unless someone manually edits constant() - which probably was the
original intent of h2xs, I do not think this argument 'arg' is ever
used.

But here it comes coupled with another bug​: exported constants are not
given prototype of "()", so

  SOME_MACRO + LOGINFO

is misparsed. See my messages on 'use autoload' on how to fix it.

Btw, when we are going to support prototype of (;$) in the lexer, so
that the core operators can be overridden?

Ilya

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 14, 2000

From [Unknown Contact. See original ticket]

In message <200003041647.LAA27286@​monk.mps.ohio-state.edu>
  Ilya Zakharevich <ilya@​math.ohio-state.edu> wrote​:

Tom Hughes writes​:

This code in the Sys​::Syslog XS backend is what h2xs generated
for me - does anyone know what the point of the second argument
to the constant() routine is given that it doesn't seem to be
used and it seems to be passed based on the first argument to
the AUTOLOAD subroutine which, as far as I know, doesn't take
any arguments...

I think the original intent was to support macros with one argument.

Right.

Well, apparently the sniffer of h2xs filters out such macros. So
unless someone manually edits constant() - which probably was the
original intent of h2xs, I do not think this argument 'arg' is ever
used.

It certainly isn't in this case so I'm going to remove it...

But here it comes coupled with another bug​: exported constants are not
given prototype of "()", so

SOME_MACRO + LOGINFO

is misparsed. See my messages on 'use autoload' on how to fix it.

Actually that doesn't seem to be the problem here. I've got a test
case now which looks like this​:

#!/usr/bin/perl -w

use Sys​::Syslog ;
Sys​::Syslog​::openlog($0, 'cons,ndelay', 'local7') ;
&syslog('info', "Test message") ;
Sys​::Syslog​::closelog() ;

The problem here is that AUTOLOAD winds up getting called to try and
load LOG_INFO and it then peeks at @​_ which is not set for AUTOLOAD subs
and hence it sees one of the callers argument arrays and passes the
nonsense value 'info' as the value of arg which the XS backend expects
to be an int.

To fix this I've removed all the unused arg junk - patch attached.

Tom

--
Tom Hughes (tom@​compton.nu)
http​://www.compton.nu/
...Things won't get any better, so get used to it.

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 14, 2000

From [Unknown Contact. See original ticket]

diff -c bleadperl/ext/Sys/Syslog/Syslog.pm perl-rc1/ext/Sys/Syslog/Syslog.pm
*** bleadperl/ext/Sys/Syslog/Syslog.pm Fri Mar 3 18​:39​:35 2000
--- perl-rc1/ext/Sys/Syslog/Syslog.pm Tue Mar 14 20​:49​:20 2000
***************
*** 122,128 ****
  our $AUTOLOAD;
  ($constname = $AUTOLOAD) =~ s/.*​:://;
  croak "& not defined" if $constname eq 'constant';
! my $val = constant($constname, @​_ ? $_[0] : 0);
  if ($! != 0) {
  croak "Your vendor has not defined Sys​::Syslog macro $constname";
  }
--- 122,128 ----
  our $AUTOLOAD;
  ($constname = $AUTOLOAD) =~ s/.*​:://;
  croak "& not defined" if $constname eq 'constant';
! my $val = constant($constname);
  if ($! != 0) {
  croak "Your vendor has not defined Sys​::Syslog macro $constname";
  }
diff -c bleadperl/ext/Sys/Syslog/Syslog.xs perl-rc1/ext/Sys/Syslog/Syslog.xs
*** bleadperl/ext/Sys/Syslog/Syslog.xs Wed Feb 16 07​:34​:16 2000
--- perl-rc1/ext/Sys/Syslog/Syslog.xs Tue Mar 14 20​:50​:58 2000
***************
*** 7,13 ****
  #endif
 
  static double
! constant_LOG_NO(char *name, int len, int arg)
  {
  switch (name[6 + 0]) {
  case 'T'​:
--- 7,13 ----
  #endif
 
  static double
! constant_LOG_NO(char *name, int len)
  {
  switch (name[6 + 0]) {
  case 'T'​:
***************
*** 36,42 ****
  }
 
  static double
! constant_LOG_N(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'D'​:
--- 36,42 ----
  }
 
  static double
! constant_LOG_N(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'D'​:
***************
*** 64,70 ****
  #endif
  }
  case 'O'​:
! return constant_LOG_NO(name, len, arg);
  }
  errno = EINVAL;
  return 0;
--- 64,70 ----
  #endif
  }
  case 'O'​:
! return constant_LOG_NO(name, len);
  }
  errno = EINVAL;
  return 0;
***************
*** 75,81 ****
  }
 
  static double
! constant_LOG_P(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'I'​:
--- 75,81 ----
  }
 
  static double
! constant_LOG_P(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'I'​:
***************
*** 104,110 ****
  }
 
  static double
! constant_LOG_AU(char *name, int len, int arg)
  {
  if (6 + 2 >= len ) {
  errno = EINVAL;
--- 104,110 ----
  }
 
  static double
! constant_LOG_AU(char *name, int len)
  {
  if (6 + 2 >= len ) {
  errno = EINVAL;
***************
*** 137,143 ****
  }
 
  static double
! constant_LOG_A(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'L'​:
--- 137,143 ----
  }
 
  static double
! constant_LOG_A(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'L'​:
***************
*** 149,155 ****
  #endif
  }
  case 'U'​:
! return constant_LOG_AU(name, len, arg);
  }
  errno = EINVAL;
  return 0;
--- 149,155 ----
  #endif
  }
  case 'U'​:
! return constant_LOG_AU(name, len);
  }
  errno = EINVAL;
  return 0;
***************
*** 160,166 ****
  }
 
  static double
! constant_LOG_CR(char *name, int len, int arg)
  {
  switch (name[6 + 0]) {
  case 'I'​:
--- 160,166 ----
  }
 
  static double
! constant_LOG_CR(char *name, int len)
  {
  switch (name[6 + 0]) {
  case 'I'​:
***************
*** 189,195 ****
  }
 
  static double
! constant_LOG_C(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'O'​:
--- 189,195 ----
  }
 
  static double
! constant_LOG_C(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'O'​:
***************
*** 201,207 ****
  #endif
  }
  case 'R'​:
! return constant_LOG_CR(name, len, arg);
  }
  errno = EINVAL;
  return 0;
--- 201,207 ----
  #endif
  }
  case 'R'​:
! return constant_LOG_CR(name, len);
  }
  errno = EINVAL;
  return 0;
***************
*** 212,218 ****
  }
 
  static double
! constant_LOG_D(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'A'​:
--- 212,218 ----
  }
 
  static double
! constant_LOG_D(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'A'​:
***************
*** 241,247 ****
  }
 
  static double
! constant_LOG_U(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'S'​:
--- 241,247 ----
  }
 
  static double
! constant_LOG_U(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'S'​:
***************
*** 270,276 ****
  }
 
  static double
! constant_LOG_E(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'M'​:
--- 270,276 ----
  }
 
  static double
! constant_LOG_E(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'M'​:
***************
*** 299,305 ****
  }
 
  static double
! constant_LOG_F(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'A'​:
--- 299,305 ----
  }
 
  static double
! constant_LOG_F(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'A'​:
***************
*** 328,334 ****
  }
 
  static double
! constant_LOG_LO(char *name, int len, int arg)
  {
  if (6 + 3 >= len ) {
  errno = EINVAL;
--- 328,334 ----
  }
 
  static double
! constant_LOG_LO(char *name, int len)
  {
  if (6 + 3 >= len ) {
  errno = EINVAL;
***************
*** 409,415 ****
  }
 
  static double
! constant_LOG_L(char *name, int len, int arg)
  {
  switch (name[5 + 0]) {
  case 'F'​:
--- 409,415 ----
  }
 
  static double
! constant_LOG_L(char *name, int len)
  {
  switch (name[5 + 0]) {
  case 'F'​:
***************
*** 421,427 ****
  #endif
  }
  case 'O'​:
! return constant_LOG_LO(name, len, arg);
  case 'P'​:
  if (strEQ(name + 5, "PR")) { /* LOG_L removed */
  #ifdef LOG_LPR
--- 421,427 ----
  #endif
  }
  case 'O'​:
! return constant_LOG_LO(name, len);
  case 'P'​:
  if (strEQ(name + 5, "PR")) { /* LOG_L removed */
  #ifdef LOG_LPR
***************
*** 440,446 ****
  }
 
  static double
! constant(char *name, int len, int arg)
  {
  errno = 0;
  if (0 + 4 >= len ) {
--- 440,446 ----
  }
 
  static double
! constant(char *name, int len)
  {
  errno = 0;
  if (0 + 4 >= len ) {
***************
*** 451,473 ****
  case 'A'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_A(name, len, arg);
  case 'C'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_C(name, len, arg);
  case 'D'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_D(name, len, arg);
  case 'E'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_E(name, len, arg);
  case 'F'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_F(name, len, arg);
  case 'I'​:
  if (strEQ(name + 0, "LOG_INFO")) { /* removed */
  #ifdef LOG_INFO
--- 451,473 ----
  case 'A'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_A(name, len);
  case 'C'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_C(name, len);
  case 'D'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_D(name, len);
  case 'E'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_E(name, len);
  case 'F'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_F(name, len);
  case 'I'​:
  if (strEQ(name + 0, "LOG_INFO")) { /* removed */
  #ifdef LOG_INFO
***************
*** 487,493 ****
  case 'L'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_L(name, len, arg);
  case 'M'​:
  if (strEQ(name + 0, "LOG_MAIL")) { /* removed */
  #ifdef LOG_MAIL
--- 487,493 ----
  case 'L'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_L(name, len);
  case 'M'​:
  if (strEQ(name + 0, "LOG_MAIL")) { /* removed */
  #ifdef LOG_MAIL
***************
*** 499,505 ****
  case 'N'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_N(name, len, arg);
  case 'O'​:
  if (strEQ(name + 0, "LOG_ODELAY")) { /* removed */
  #ifdef LOG_ODELAY
--- 499,505 ----
  case 'N'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_N(name, len);
  case 'O'​:
  if (strEQ(name + 0, "LOG_ODELAY")) { /* removed */
  #ifdef LOG_ODELAY
***************
*** 511,517 ****
  case 'P'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_P(name, len, arg);
  case 'S'​:
  if (strEQ(name + 0, "LOG_SYSLOG")) { /* removed */
  #ifdef LOG_SYSLOG
--- 511,517 ----
  case 'P'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_P(name, len);
  case 'S'​:
  if (strEQ(name + 0, "LOG_SYSLOG")) { /* removed */
  #ifdef LOG_SYSLOG
***************
*** 523,529 ****
  case 'U'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_U(name, len, arg);
  case 'W'​:
  if (strEQ(name + 0, "LOG_WARNING")) { /* removed */
  #ifdef LOG_WARNING
--- 523,529 ----
  case 'U'​:
  if (!strnEQ(name + 0,"LOG_", 4))
  break;
! return constant_LOG_U(name, len);
  case 'W'​:
  if (strEQ(name + 0, "LOG_WARNING")) { /* removed */
  #ifdef LOG_WARNING
***************
*** 629,643 ****
 
 
  double
! constant(sv,arg)
  PREINIT​:
  STRLEN len;
  INPUT​:
  SV * sv
  char * s = SvPV(sv, len);
- int arg
  CODE​:
! RETVAL = constant(s,len,arg);
  OUTPUT​:
  RETVAL
 
--- 629,642 ----
 
 
  double
! constant(sv)
  PREINIT​:
  STRLEN len;
  INPUT​:
  SV * sv
  char * s = SvPV(sv, len);
  CODE​:
! RETVAL = constant(s,len);
  OUTPUT​:
  RETVAL
 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant