pkg/46713: comm/p5-Device-SerialPort has 'status()' unnecessarily disabled

[m.ramakers%gmail.com@localhost]

>Number:         46713
>Category:       pkg
>Synopsis:       comm/p5-Device-SerialPort has 'status()' unnecessarily disabled
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    pkg-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jul 17 14:50:00 +0000 2012
>Originator:     Michai Ramakers
>Release:        6.0 BETA2
>Organization:
>Environment:
NetBSD dwarf 6.0_BETA2 NetBSD 6.0_BETA2 (MIC) #1: Sun Jun 24 22:05:29 CEST 2012 
 michai@dwarf:/usr/src/sys/arch/i386/compile/MIC i386
>Description:
[ this is a verbatim copy of report upstream: 
https://rt.cpan.org/Public/Bug/Display.html?id=78431 ]


ENVIRONMENT:

tested on NetBSD 6.0BETA2 / i386 and Device::SerialPort 1.04. I did not
test on Linux, although I think behaviour would be ok (see 'ANALYSIS':
ioctl 'TIOCINQ' is present in bits/ioctls.h on Linux systems, afaik).


DESCRIPTION:

'Device::SerialPort::status()' is inconveniently disabled as per
'can_status()', making it necessary to use 'Device::SerialPort::ioctl(
"TIOCOUTQ", \$num_byte )' to read the number of bytes pending write in
the output-buffer. 

If not necessary, it would IMHO be better to do this without explicit
ioctls.


ANALYSIS:

Implementation of 'status()' (SerialPort.pm):

  1870 sub status {
  1871     my $self = shift;
  1872     return if (@_);
  1873     return unless ($self->can_status);

(i.e. precondition for this function to work is 'can_status()' returning
true)

Implementation of 'can_status()' (SerialPort.pm):

   702 sub can_status {
   703     return 1 if (defined($bits->{'portable_TIOCINQ'}) &&
   704                  defined($bits->{'TIOCOUTQ'}));
   705     return 0;
   706     #return 0 unless ($incount && $outcount);
   707     #return 1;
   708 }

On NetBSD, 'TIOCOUTQ' is found at compile-time (namely, in
/usr/include/sys/ttycom.h).

A pseudo-flag 'portable_TIOCINQ' is used instead of 'TIOCINQ'
(SerialPort.pm):

    91 # Set alternate bit names
    92 $bits->{'portable_TIOCINQ'} = $bits->{'TIOCINQ'} ||
$bits->{'FIONREAD'};

This is fine, since TIOCINQ and FIONREAD are the same, from what I
understood.

On NetBSD, TIOCINQ is not found at compile-time, but FIONREAD is
(namely, in /usr/include/sys/filio.h). 

The '$bits' hash is constructed in the build-process (SerialPort.xs):

   99 #ifdef TIOCINQ
  100   ADD_TO_HASH(TIOCINQ)
  101 #endif
  102 #ifdef TIOCOUTQ
  103   ADD_TO_HASH(TIOCOUTQ)
  104 #endif

However, a similar clause for 'FIONREAD' is not present - it seems the
fallback to 'FIONREAD' as substitute to 'TIOCINQ' was not completely
implemented.


WORKAROUND:

to get number of bytes in output-buffer, use
'Device::SerialPort::ioctl()' as alternative, as stated in the manual:

  my $mstat = " ";
  $my_serial_port->ioctl( 'TIOCOUTQ', \$mstat ) or die;
  my $count = unpack( 'L', $mstat );
  print "there are $count bytes pending write\n";

>How-To-Repeat:
    1 #!/usr/pkg/bin/perl
    2 
    3 use strict;
    4 use warnings;
    5 use Device::SerialPort;
    6 
    7 my $port = Device::SerialPort->new( "/dev/ttyU0" ) or die;
    8 $port->can_status() or die;   # on NetBSD, execution fails here

(alternatively, list known ioctls using a oneliner - see suggested fix.)

>Fix:
add a clause for 'FIONREAD' to SerialPort.xs:

  #ifndef FIONREAD
    ADD_TO_HASH(FIONREAD)
  #endif

To verify effect of fix without actually opening a port:

$ perl -MDevice::SerialPort -MData::Dumper -e 'print
Data::Dumper::Dumper Device::SerialPort::Bits::get_hash()' | grep FIONREAD

...returns nothing before, and returns 1 entry after the fix. 

I also verified the functionality of 'status()' after the fix w.r.t.
bytes in output-buffer, which worked fine.