Re: [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs

On 17 July 2013 03:11, Tony Cook via RT <perlbug-followup@perl.org> wrote:
> On Tue Jul 09 22:22:37 2013, tonyc wrote:
>> On Mon Jul 01 17:35:57 2013, tonyc wrote:
>> > On Tue, Jun 25, 2013 at 03:57:04PM +0200, Slaven Rezic wrote:
>> > There's one bug I can see with this change:
>> >
>> >   $ ./perl -Ilib -MData::Dumper -e '++$Data::Dumper::Useqq; print
>> > Dumper("\x011")'
>> >   $VAR1 = "\11";
>> >
>> > This isn't present in the perl version:
>> >
>> >   $ ./perl -Ilib -MData::Dumper -e '++$Data::Dumper::Useqq;
>> > ++$Data::Dumper::Useperl; print Dumper("\x011")'
>> >   $VAR1 = "\0011";
>>
>> I've attached a new series of patches:
>>
>> 0001 - Slaven's patch, but don't make dumper.t +x
>> 0002 - match whitespace to the original code
>> 0003 - fix the problem described above
>
> Applied as:
> 9baac1a3613bd641a847683d7877b3cfab3244bc,
> f1c459431a0f7a1dabba8b8a2b063a110f2f6284,
> dbf00f6932923fb26983322f5049702fc937a399 and
> merge commit 2780a6ede39cb2f190256ad43e1c61535103caf9.

I dont think this patch can be right (I have not verified) and I dont
see tests for behavior I expect needs testing:

 #else
+      if (useqq && (k <= 31 || k == 127 || (!do_utf8 && k > 127))) {
+ *r++ = '\\';
+ switch (k) {
+    case 7:  *r++ = 'a'; break;
+    case 8:  *r++ = 'b'; break;
+    case 9:  *r++ = 't'; break;
+    case 10: *r++ = 'n'; break;
+    case 12: *r++ = 'f'; break;
+    case 13: *r++ = 'r'; break;
+    case 27: *r++ = 'e'; break;
+                    default:
+                      /* faster than
+                       * r = r + my_sprintf(r, "%o", k);
+                       */
+                      if (k <= 7) {
+                        *r++ = (char)k + '0';
+                      } else if (k <= 63) {
+                        *r++ = (char)(k>>3) + '0';
+                        *r++ = (char)(k&7) + '0';
+                      } else {
+                        *r++ = (char)(k>>6) + '0';
+                        *r++ = (char)((k&63)>>3) + '0';
+                        *r++ = (char)(k&7) + '0';
+      }
+ }

I see nothing that deals with something like this: "\0000"

That is, a string containing an octal null followed by a zero.

Or similar cases. The pureperl implementation DOES deal with this:

$ perl -MData::Dumper -e'$Data::Dumper::Useqq=1; print Dumper("\0000")'
$VAR1 = "\0000";

Now, I admit I havent *tried* the patch, but I did not see any code to
look ahead at the *next* character and check if it is an octal digit,
and if so force "three digit octal" mode.

Yves


--
perl -Mre=debug -e "/just|another|perl|hacker/"

• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by Tony Cook via RT
• Re: [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by demerphq
• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by Tony Cook via RT
• Re: [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by demerphq
• Re: [perl #74798] [PATCH] Data::Dumper: useqq implementation forxs by Slaven Rezic
• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by Father Chrysostomos via RT
• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by James E Keenan via RT
• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by Tony Cook via RT
• [perl #74798] [PATCH] Data::Dumper: useqq implementation for xs by Tony Cook via RT