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

[PATCH] hex() documentation not clear on invalid input #15006

Closed
p5pRT opened this issue Oct 23, 2015 · 21 comments
Closed

[PATCH] hex() documentation not clear on invalid input #15006

p5pRT opened this issue Oct 23, 2015 · 21 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 23, 2015

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

Searchable as RT126437$

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From @epa

Created by @epa

The documentation in perlfunc for C<hex> says nothing about the
behaviour when the string passed isn't a valid hex number. What seems
to happen is that it takes the longest initial substring of the input
that is a valid hex number (counting the empty string as valid). If
warnings are enabled, you see "Illegal hexadecimal digit 'z' ignored",
but only for the first bad character.

If there is a consensus that this is the correct and intended
behaviour, then it should be documented (I'll be happy to write
something). Myself, I would greatly prefer hex() to return undef if
the input isn't a valid hex number, but it may be too late to change.

(If the current behaviour is kept, is there any way to test whether
the hex() call succeeded without problems? Other than by setting a
__WARN__ handler. If there is such a trick, it would be worth
documenting too.)

Perl Info

Flags:
    category=docs
    severity=low

Site configuration information for perl 5.20.3:

Configured by Red Hat, Inc. at Thu Sep 24 08:45:26 UTC 2015.

Summary of my perl5 (revision 5 version 20 subversion 3) configuration:
   
  Platform:
    osname=linux, osvers=4.1.6-100.fc21.x86_64, archname=x86_64-linux-thread-multi
    uname='linux buildvm-04.phx2.fedoraproject.org 4.1.6-100.fc21.x86_64 #1 smp mon aug 17 22:20:37 utc 2015 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches  -m64 -mtune=generic -Wl,-z,relro  -Dshrpdir=/usr/lib64 -DDEBUGGING=-g -Dversion=5.20.3 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='5.1.1 20150618 (Red Hat 5.1.1-4)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib64 /lib64 /usr/lib64 /usr/local/lib /usr/lib /lib/../lib64 /usr/lib/../lib64 /lib
    libs=-lpthread -lresolv -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -lresolv -lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.21.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.21'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,--enable-new-dtags'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m64 -mtune=generic -Wl,-z,relro  -L/usr/local/lib'

Locally applied patches:
    Fedora Patch1: Removes date check, Fedora/RHEL specific
    Fedora Patch3: support for libdir64
    Fedora Patch4: use libresolv instead of libbind
    Fedora Patch5: USE_MM_LD_RUN_PATH
    Fedora Patch6: Skip hostname tests, due to builders not being network capable
    Fedora Patch7: Dont run one io test due to random builder failures
    Fedora Patch15: Define SONAME for libperl.so
    Fedora Patch16: Install libperl.so to -Dshrpdir value
    Fedora Patch22: Document Math::BigInt::CalcEmu requires Math::BigInt (CPAN RT#85015)
    Fedora Patch25: Use stronger algorithm needed for FIPS in t/op/crypt.t (RT#121591)
    Fedora Patch26: Make *DBM_File desctructors thread-safe (RT#61912)
    Fedora Patch27: Report inaccesible file on failed require (RT#123270)
    Fedora Patch28: Use stronger algorithm needed for FIPS in t/op/taint.t (RT#123338)
    Fedora Patch29: Fix debugger y command scope level
    Fedora Patch200: Link XS modules to libperl.so with EU::CBuilder on Linux
    Fedora Patch201: Link XS modules to libperl.so with EU::MM on Linux


@INC for perl 5.20.3:
    /home/eda/share/perl5
    /home/eda/lib/perl5/
    /home/eda/lib64/perl5/
    /usr/local/lib64/perl5
    /usr/local/share/perl5
    /usr/lib64/perl5/vendor_perl
    /usr/share/perl5/vendor_perl
    /usr/lib64/perl5
    /usr/share/perl5
    .


Environment for perl 5.20.3:
    HOME=/home/eda
    LANG=en_GB.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LC_CTYPE=en_GB.UTF-8
    LC_MESSAGES=en_GB.UTF-8
    LC_MONETARY=en_GB.UTF-8
    LC_NUMERIC=en_GB.UTF-8
    LC_TIME=en_GB.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/eda/bin:/home/eda/bin:/usr/local/bin:/usr/bin:/sbin:/usr/sbin:/sbin:/usr/sbin
    PERL5LIB=/home/eda/share/perl5:/home/eda/lib/perl5/:/home/eda/lib64/perl5/
    PERL_BADLANG (unset)
    SHELL=/bin/bash

This email is intended only for the person to whom it is addressed and may contain confidential information. Any retransmission, copying, disclosure or other use of, this information by persons other than the intended recipient is prohibited. If you received this email in error, please contact the sender and delete the material. This email is for information only and is not intended as an offer or solicitation for the purchase or sale of any financial instrument. Wadhwani Asset Management LLP is a Limited Liability Partnership registered in England (OC303168) with registered office at 40 Berkeley Square, 3rd Floor, London, W1J 5AL. It is authorised and regulated by the Financial Conduct Authority.

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From zefram@fysh.org

Ed Avis wrote​:

(If the current behaviour is kept, is there any way to test whether
the hex() call succeeded without problems?

You can always check /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/. Generally hex()
is best used when by the nature of the way you acquired the input you
already know it consists only of hex digits.

-zefram

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From @epa

Zefram <zefram <at> fysh.org> writes​:

(If the current behaviour is kept, is there any way to test whether
the hex() call succeeded without problems?

You can always check /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/. Generally hex()
is best used when by the nature of the way you acquired the input you
already know it consists only of hex digits.

That's right, and of course I am doing that.

Ah - and I see from your regexp that hex() does understand the 0x prefix,
which from a first reading of the documentation I assumed it wouldn't, since
only 'oct' coped with prefixes like 0, 0x, 0b... it is a bit ambiguous.

Generally even if I have extracted data with a regexp I still want to check
that parsing has succeeded, although as a 'cannot happen' condition it would
just get a simple die; if parsing failed. This is just good practice in my
view, and stops a bug in one regexp cascading through the program.
So C<hex($x) // die> would be most convenient.

--
Ed Avis <eda@​waniasset.com>

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From Eirik-Berg.Hanssen@allverden.no

On Fri, Oct 23, 2015 at 6​:01 PM, Ed Avis <perlbug-followup@​perl.org> wrote​:

(If the current behaviour is kept, is there any way to test whether
the hex() call succeeded without problems? Other than by setting a
__WARN__ handler. If there is such a trick, it would be worth
documenting too.)

  I'm not sure about documenting it, but …

  eval { use warnings "FATAL"; hex $foo }

  Though personally, I guess I'd trim (or at least chomp()) $foo first​:

$ perl -E 'say eval { use warnings "FATAL"; hex(s/^\s+|\s+$//rg) } //
"undef​: $@​" for @​ARGV' deadbeef "" bar " 1a
"
3735928559
0
undef​: Illegal hexadecimal digit 'r' ignored at -e line 1.

26
$

Eirik

@p5pRT
Copy link
Author

p5pRT commented Oct 23, 2015

From Eirik-Berg.Hanssen@allverden.no

On Fri, Oct 23, 2015 at 6​:24 PM, Ed Avis <eda@​waniasset.com> wrote​:

So C<hex($x) // die> would be most convenient.

  do { use warnings "FATAL"; hex($x) } # ;-)

Eirik

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2015

From @epa

Proposed patch codifying the current behaviour​:

index f0a2abb..1811d8b 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@​@​ -3031,16 +3031,31 @​@​ X<hex> X<hexadecimal>
=for Pod​::Functions convert a string to a hexadecimal number

Interprets EXPR as a hex string and returns the corresponding value.
-(To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
-L</oct>.) If EXPR is omitted, uses C<$_>.
+An initial C<0x> prefix is stripped. (See L</oct> for a way to automatically
+convert strings starting with any of C<0>, C<0x>, or C<0b>.)
+If EXPR is omitted, uses C<$_>.

  print hex '0xAf'; # prints '175'
  print hex 'aF'; # same

Hex strings may only represent integers. Strings that would cause
-integer overflow trigger a warning. Leading whitespace is not stripped,
-unlike oct(). To present something as hex, look into L</printf>,
-L</sprintf>, and L</unpack>.
+integer overflow trigger a warning.
+
+If the input is not a valid hex string then C<hex> takes the longest
+prefix of the input that is a valid hex string and converts that. A warning
+is given for the first invalid character seen. So if you want to make
+sure that C<hex> succeeds, you can either trap the warning​:
+
+ $v = do { use warnings 'FATAL'; hex $x };
+
+or alternatively check the input by hand first​:
+
+ $x =~ /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/
+ or die "bad hex string $x";
+ $v = hex $x;
+
+Leading whitespace is not stripped, unlike oct(). To present
+something as hex, look into L</printf>, L</sprintf>, and L</unpack>.

=item import LIST
X<import>

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2015

From @epa

...another question is to what extent 'x' instead of '0x' is supported? Should the docs for both hex and oct change to list both variants? Ditto 'b' vs '0b'.

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2015

From @Abigail

On Mon, Oct 26, 2015 at 08​:10​:34AM -0700, Ed Avis via RT wrote​:

Proposed patch codifying the current behaviour​:

index f0a2abb..1811d8b 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@​@​ -3031,16 +3031,31 @​@​ X<hex> X<hexadecimal>
=for Pod​::Functions convert a string to a hexadecimal number

Interprets EXPR as a hex string and returns the corresponding value.
-(To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
-L</oct>.) If EXPR is omitted, uses C<$_>.
+An initial C<0x> prefix is stripped. (See L</oct> for a way to automatically
+convert strings starting with any of C<0>, C<0x>, or C<0b>.)
+If EXPR is omitted, uses C<$_>.

 print hex '0xAf'; \# prints '175'
 print hex 'aF';   \# same

Hex strings may only represent integers. Strings that would cause
-integer overflow trigger a warning. Leading whitespace is not stripped,
-unlike oct(). To present something as hex, look into L</printf>,
-L</sprintf>, and L</unpack>.
+integer overflow trigger a warning.
+
+If the input is not a valid hex string then C<hex> takes the longest
+prefix of the input that is a valid hex string and converts that. A warning
+is given for the first invalid character seen. So if you want to make
+sure that C<hex> succeeds, you can either trap the warning​:
+
+ $v = do { use warnings 'FATAL'; hex $x };
+
+or alternatively check the input by hand first​:
+
+ $x =~ /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/
+ or die "bad hex string $x";
+ $v = hex $x;
+
+Leading whitespace is not stripped, unlike oct(). To present
+something as hex, look into L</printf>, L</sprintf>, and L</unpack>.

I find the phrase "If you want to make sure that hex succeeds..."
confusing. Both code snippets you present do exactly the opposite​:
they die.

I'd also use "ignored" where you say "stripped". Because in

  my $hex = "0xF00";
  say hex $hex;
  say $hex;

the second line still prints the leading '0x', and nothing gets
stripped.

Abigail

@p5pRT
Copy link
Author

p5pRT commented Oct 26, 2015

From @epa

Thanks for your comments; here's a revised patch. Note that perlfunc often uses 'stripped' to mean that certain data is ignored and not included in the output of a function, without implying that the function destructively modifies its input. Within the documentation for 'hex', I have standardized on 'ignored' as you suggest.

--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@​@​ -3031,16 +3031,40 @​@​ X<hex> X<hexadecimal>
=for Pod​::Functions convert a string to a hexadecimal number

Interprets EXPR as a hex string and returns the corresponding value.
-(To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
-L</oct>.) If EXPR is omitted, uses C<$_>.
+A hex string has an optional C<x> or C<0x> prefix, then consists of
+digits C<[0-9A-Fa-f]>. Optional C<_> characters may appear among
+the digits purely for padding.
+
+(See L</oct> for a way to automatically convert strings starting with
+any of C<0>, C<0x>, or C<0b>.) If EXPR is omitted, uses C<$_>.

  print hex '0xAf'; # prints '175'
  print hex 'aF'; # same

Hex strings may only represent integers. Strings that would cause
-integer overflow trigger a warning. Leading whitespace is not stripped,
-unlike oct(). To present something as hex, look into L</printf>,
-L</sprintf>, and L</unpack>.
+integer overflow trigger a warning.
+
+If the input is not a valid hex string then C<hex> takes the longest
+prefix of the input that is a valid hex string and converts that. A
+warning is given for the first invalid character seen. If you want to
+make sure that hex decoded the whole input string successfully, you
+can trap the warning​:
+
+ $v = do { use warnings 'FATAL'; hex $x };
+
+This will give a fatal error if the input was not a valid hex string,
+and also if it was valid but gave an integer so large it overflowed.
+
+Alternatively you can check the input by hand first​:
+
+ $x =~ /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/
+ or die "bad hex string $x";
+ $v = hex $x;
+
+But here, integer overflow could still have occurred.
+
+Leading whitespace is not ignored, unlike oct(). To present
+something as hex, look into L</printf>, L</sprintf>, and L</unpack>.

=item import LIST
X<import>

@p5pRT
Copy link
Author

p5pRT commented Nov 2, 2015

From @tonycoz

On Fri Oct 23 09​:01​:10 2015, eda@​waniasset.com wrote​:

The documentation in perlfunc for C<hex> says nothing about the
behaviour when the string passed isn't a valid hex number. What seems
to happen is that it takes the longest initial substring of the input
that is a valid hex number (counting the empty string as valid). If
warnings are enabled, you see "Illegal hexadecimal digit 'z' ignored",
but only for the first bad character.

The behaviour is inconsistent - it croaks if the string contains a wide character​:

tony@​mars​:.../git/perl$ ./perl -le 'print hex("x20_00\x{101}")'
Wide character in hex at -e line 1.

which is probably a bug.

Tony

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @epa

The behaviour is inconsistent - it croaks if the string contains a
wide character​:

Yet this appears deliberate​: there is a test in t/oct.t which makes sure that wide characters cause an exception. On the other hand there are no test cases for ASCII, but non-hex-digit characters in the string.

Should the behaviour be made consistent so that all bad characters, wide or not, throw an exception? Or should it be that all bad characters just raise a warning and the longest valid initial substring is taken as the hex input? Or do we just codify and test the current somewhat wonky semantics?

If the perl5-porters can indicate the preferred way forward then I will include some test cases as well as the documentation change.

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @ap

* Ed Avis via RT <perlbug-followup@​perl.org> [2015-10-26 16​:45]​:

Thanks for your comments; here's a revised patch.

This triples the length of the entry. Ignoring the wide character bug,
would the docs have satisfied you if they had been as in this patch?

Inline Patch
diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index f0a2abbb4c..00332d8fbd 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -3028,18 +3028,23 @@ X<hex> X<hexadecimal>

 =item hex

-=for Pod::Functions convert a string to a hexadecimal number
+=for Pod::Functions convert a hexadecimal string to a number

-Interprets EXPR as a hex string and returns the corresponding value.
-(To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
-L</oct>.)  If EXPR is omitted, uses C<$_>.
+Interprets EXPR as a hex string and returns the corresponding numeric value.
+If EXPR is omitted, uses C<$_>.

     print hex '0xAf'; # prints '175'
     print hex 'aF';   # same

-Hex strings may only represent integers.  Strings that would cause
-integer overflow trigger a warning.  Leading whitespace is not stripped,
-unlike oct().  To present something as hex, look into L</printf>,
+A hex string consists of hex digits and an optional C<0x> or C<x> prefix.
+Each hex digit may be preceded by a single underscore, which will be ignored.
+Any other character triggers a warning and causes the rest of the string
+to be ignored (even leading whitespace, unlike L</oct>).
+Only integers can be represented, and integer overflow triggers a warning.
+
+To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
+L</oct>.
+To present something as hex, look into L</printf>,
 L</sprintf>, and L</unpack>.

 =item import LIST

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @epa

I like your more concise wording. But I think it is useful to have Zefram's regexp of valid hex numbers and an idiot-proof example of how to use it; or alternatively the example of how to check that hex() succeeded by trapping warnings. Perhaps it is overkill to mention both, but one or the other should be there. Just as much of the documentation on open() talks about how to make sure it succeeded, so hex() needs to suggest and encourage good error checking practices.

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @cowens

On Tue, Nov 3, 2015 at 7​:36 AM Ed Avis via RT <perlbug-followup@​perl.org> wrote​:

I like your more concise wording. But I think it is useful to have Zefram's regexp of valid hex numbers and an idiot-proof example of how to use it; or alternatively the example of how to check that hex() succeeded by trapping warnings. Perhaps it is overkill to mention both, but one or the other should be there. Just as much of the documentation on open() talks about how to make sure it succeeded, so hex() needs to suggest and encourage good error checking practices.

---
via perlbug​: queue​: perl5 status​: open
https://rt-archive.perl.org/perl5/Ticket/Display.html?id=126437

Here is my take. It includes all of the variant headers and examples
of each (using a loop to save space).

I also stripped the statement that only integers can be represented
(the lack of . character means you can't represent real numbers and
the integer overflow warning should be enough to let the user know the
backend type used to represent the number).

Inline Patch
diff --git a/pod/perlfunc.pod b/pod/perlfunc.pod
index f0a2abb..3babe72 100644
--- a/pod/perlfunc.pod
+++ b/pod/perlfunc.pod
@@ -3028,19 +3028,27 @@ X<hex> X<hexadecimal>

 =item hex

-=for Pod::Functions convert a string to a hexadecimal number
+=for Pod::Functions convert a hex string to a number

-Interprets EXPR as a hex string and returns the corresponding value.
-(To convert strings that might start with either C<0>, C<0x>, or C<0b>, see
-L</oct>.)  If EXPR is omitted, uses C<$_>.
+Interprets EXPR as a hex string and returns the corresponding decimal value.
+If EXPR is omitted, uses C<$_>.
+
+    # All of these conversions are equivalent
+    for my $hex_string (qw/ 0xAf 0XAf xaF XaF aF 0x_a_f /) {
+        print hex($hex_string), "\n";
+    }
+
+A hex string consists of an optional header (C<0x|0X|x|X>), hexadecimal
+digits (C<[0-9a-fA-F]>), and optional digit separators (one C<_> is allowed
+before each digit).  Any other character triggers a warning and causes the rest
+of the string to be ignored (even leading whitespace, unlike L<oct>).  Strings
+that would cause integer overflow trigger a warning.

-    print hex '0xAf'; # prints '175'
-    print hex 'aF';   # same
+To convert strings that might start with one of C<0> (octal), C<0x> (hex),
+or C<0b> (binary), see L</oct>.

-Hex strings may only represent integers.  Strings that would cause
-integer overflow trigger a warning.  Leading whitespace is not stripped,
-unlike oct().  To present something as hex, look into L</printf>,
-L</sprintf>, and L</unpack>.
+To convert a number into hex, look into L</printf>, L</sprintf>, and
+L</unpack>.

 =item import LIST
 X<import>

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @ap

* Ed Avis via RT <perlbug-followup@​perl.org> [2015-11-03 13​:40]​:

I like your more concise wording. But I think it is useful to have
Zefram's regexp of valid hex numbers and an idiot-proof example of how
to use it; or alternatively the example of how to check that hex()
succeeded by trapping warnings. Perhaps it is overkill to mention
both, but one or the other should be there. Just as much of the
documentation on open() talks about how to make sure it succeeded, so
hex() needs to suggest and encourage good error checking practices.

Actually, you’re right.

I don’t think an explanation of the fact that you can fatalise warnings
belongs here. Do we add that to every function that can throw warnings?
If not, what makes hex() different? What level of familiarity with which
parts of Perl can/should/do we expect from a reader, and why?

But even for the regexp I was reluctant, since it seemed redundant with
the prose specification (and I think it’s important that there be a spec
in precise prose). However, it does make sense not to make every reader
grok the spec from scratch and figure out how to write that (slightly
tricky) pattern. That information is also not redundant with other parts
of the documentation nor is it just a basic language feature.

So I went to propose adding just this line to the code examples (to keep
things brief)​:

  $valid_input =~ /\A(?​:0?[xX])?(?​:_?[0-9a-fA-F])*\z/

But when I tried that on for size, I realised that the prose and pattern
are actually complementary​: each explains the other. So I now think that
that *has* to be there.

Does that work for you?

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @epa

It depends on whether you see 'perlfunc' as reference or tutorial. As a reference or spec for Perl's builtin functions, it certainly should not repeat the same information about fatal warnings in random places. If a tutorial, then the emphasis should be on examples and reminding the reader of useful related information, even if that causes some redundancy. 'perlopentut' has lots of repeated boilerplate showing how to use die and $! even though in principle it would be enough to mention just once that open() returns false on failure and sets $!.

If the regexp is included then that takes care of showing how to check that hex() will succeed, so an example of trapping warnings from it is less necessary. (It might be useful for that regexp itself to be provided in core, or for core to have a looks_like_hex() builtin... but that is for another day.)

@p5pRT
Copy link
Author

p5pRT commented Nov 3, 2015

From @ap

* Chas. Owens <chas.owens@​gmail.com> [2015-11-03 15​:25]​:

Here is my take. It includes all of the variant headers and examples
of each (using a loop to save space).

Following the principle that they call “lots helps lots” in Germany? :-)

I also stripped the statement that only integers can be represented
(the lack of . character means you can't represent real numbers and
the integer overflow warning should be enough to let the user know the
backend type used to represent the number).

I considered that but decided to keep an explicit mention because Perl
does now have hexadecimal floating point literals – which hex() doesn’t
support. Just 5 words extra anyway.

Other quibbles​:

• You dropped the pointer that oct() is capable of parsing non-oct
  numbers based on prefix, which I think makes a lot of sense to include
  here. (I chose to move it to an crossrefs para at the bottom.)

• Minor​: you have L<oct> in there; there is no such document to link to.
  You meant L</oct>.

Regards,
--
Aristotle Pagaltzis // <http​://plasmasturm.org/>

@p5pRT
Copy link
Author

p5pRT commented Nov 23, 2015

From @ap

* Ed Avis via RT <perlbug-followup@​perl.org> [2015-11-03 19​:15]​:

'perlopentut' has lots of repeated boilerplate showing how to use die
and $! even though in principle it would be enough to mention just
once that open() returns false on failure and sets $!.

But perldoc -f open is much less of a tutorial and we’re not talking
about perlhextut in this ticket. (Not to mention I would consider the
function a disaster if we had to have such a document.)

If the regexp is included then that takes care of showing how to check
that hex() will succeed, so an example of trapping warnings from it is
less necessary.

I’ve just pushed that patch as fc61cbf.

@p5pRT
Copy link
Author

p5pRT commented Dec 6, 2015

From @jkeenan

Patch was applied by Aristotle; closing.
--
James E Keenan (jkeenan@​cpan.org)

@p5pRT
Copy link
Author

p5pRT commented Dec 6, 2015

@jkeenan - Status changed from 'open' to 'resolved'

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

No branches or pull requests

1 participant