http://qs321.pair.com?node_id=11142849


in reply to BackupPC or Data::Dumper playing foul...?

It would help if you could define what you mean by "works fine" and "fails", ideally by giving an sscce.

According to the docs, the intention when supplying [*value] is that the output will assign a name appropriate to the type of the value: if you give it a hashref to dump, it will assume it actually represents a hash called %value, for example. It appears this has been broken for a long time, and was recently fixed - a quick scan of the Changes file (https://metacpan.org/dist/Data-Dumper/changes) suggests it may have been the very recent 2.182_51 change.

With latest bleadperl (which has v2.184, not yet available on CPAN), I get:

% perl -MData::Dumper -e '
  my %h = (one => 1);
  $h{refone} = \$h{one};
  # make it a self-referential structure
  my $d = Data::Dumper->new([\%h], [*value]);
  $d->Indent(1);
  $d->Terse(1);
  $d->Sortkeys(1);
  print $d->Dump;
'
(
  'one' => 1,
  'refone' => \$main::value{'one'}
)

[download]

.. which I believe is the correct intended output. With older versions (back at least as far as maint-5.8) I get instead:

{
  'one' => 1,
  'refone' => \$VAR1->{'one'}
}

[download]

.. which is wrong, since it refers to $VAR1 instead of to %value.

By changing it to \[*value] I guess you have turned it into something Dump() does not recognise, so it ignores it. The result is that you get the same output as the previous broken behaviour. You can achieve the same by removing the [*value] arrayref altogether.

(I'm slightly worried that the changelog states that this was a fix only for the XS code: additionally calling $d->Useperl(1) before the Dump() call does not change the output in either case for me, so it's possible I'm completely misunderstanding what's going on.)

Hugo

Replies are listed 'Best First'.
Re^2: BackupPC or Data::Dumper playing foul...?
by Krambambuli (Curate) on Apr 08, 2022 at 18:20 UTC
    I'm pretty sure it is related to the Changes that you are referring to.
    Simple code to play with and check the issue:

    #!/usr/bin/perl

use strict;
use warnings;

use Data::Dumper;

my $newConf =  { 'FullKeepCnt' => [ 4 ] };

print "Original structure: ", Dumper( $newConf );

foreach my $var ( sort(keys(%$newConf)) ) {
    my $d1 = Data::Dumper->new([$newConf->{$var}], \[*value]);
    print "\nd1: ", Dumper( $d1 );
    my $value1 = $d1->Dump;
    print "\nvalue 1: $value1\n";

    my $d2 = Data::Dumper->new([$newConf->{$var}], [*value]);
    $d2->Useperl(0);
    print "\nd2: ", Dumper( $d2 );
    my $value2 = $d2->Dump;
    print "\nvalue2: $value2\n";
}

    [download]
    The output that matters is 
    value2: @main::value = (
                 4
               );

    [download]
    whereas the initial arrayref is what BackupPC needs and expects. (The list it gets back now via DumpXS actually ruins BackupPCs config files when using the web interface for editing...)

    The pure Perl version returns a list, and so I guess that that's what it should be. My current understanding is that BackupPC did it wrongly for many years, and still does, but was able to get away with it because DumpXS within Data::Dumper was buggy. Now that Data::Dumper is corrected, BackupPCs needs to patch it's code.

    Afaict, \[*value] works fine, both with DumpXS and pure Perl. But I'm not fully sure that that's 'the best' patch to be done, even if it works for me.

[reply]
[d/l]
[select]

      Well this all depends on how the output from Dump (or Dumper) is going to be used, which you don't show; but yes, if the intent is to eval() the output to store in a scalar, passing [*value] is exactly the wrong thing to do.

      Even if \[*value] works for you now, it is an invalid parameter for Data::Dumper, so it's likely that another future version will actually check that and complain about it. The correct fix, as I mentioned in my original reply, is to remove the second parameter to new() altogether: just make it Data::Dumper->new([$newConf->{$var}]).

[reply]
[d/l]
[select]
        Took me a while to understand and check. Indeed, omitting the second argument entirely seems to be the correct patch.

        I'm not fully sure yet how the output is used; BackupPC's source is not necessarily easy to read/follow and I'm not at all 'fluent' in it. But as the changes to config values (done via CGI) are merged with the values read in from the old config files, as strings into executable config.pl files, there sure is some later eval() somewhere.

        Thank you!

[reply]

In Section Seekers of Perl Wisdom