| Message ID | 1512382841.17323.11.camel@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Dec 4, 2017 at 12:20 PM, <kaiwan.billimoria@gmail.com> wrote: > On Mon, 2017-12-04 at 19:21 +1100, Tobin C. Harding wrote: >> On Mon, Dec 04, 2017 at 10:51:53AM +0530, Kaiwan N Billimoria wrote: >> > > --- >> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl >> > index 9906dcf8b807..260b52e456f1 100755 >> > --- a/scripts/leaking_addresses.pl >> > +++ b/scripts/leaking_addresses.pl >> > @@ -266,7 +266,7 @@ sub is_false_positive >> > sub is_false_positive_ix86_32 >> > { >> > my ($match) = @_; >> > - state $page_offset = eval get_page_offset(); # only gets called once >> > + state $page_offset = hex get_page_offset(); # only gets called once >> >> I don't think this is valid ;) I meant use hex() to convert the string >> to an int so it doesn't throw the warning (inside get_page_offset()). > > Yup, got it, thanks :-p > Combined patch below: > > > --- > scripts/leaking_addresses.pl | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl > index 9906dcf8b807..a595a2c66b12 100755 > --- a/scripts/leaking_addresses.pl > +++ b/scripts/leaking_addresses.pl > @@ -266,8 +266,7 @@ sub is_false_positive > sub is_false_positive_ix86_32 > { > my ($match) = @_; > - state $page_offset = eval get_page_offset(); # only gets called once > - > + state $page_offset = get_page_offset(); # only gets called once > if ($match =~ '\b(0x)?(f|F){8}\b') { > return 1; > } > @@ -283,7 +282,7 @@ sub is_false_positive_ix86_32 > sub get_page_offset > { > my $page_offset; > - my $default_offset = "0xc0000000"; > + my $default_offset = hex("0xc0000000"); > my @config_files; > > # Allow --page-offset-32bit to override. > @@ -306,23 +305,23 @@ sub get_page_offset > } else { > $page_offset = parse_kernel_config_file($tmp_file); > if ($page_offset ne "") { > - return $page_offset; > + return hex($page_offset); > } > } > system("rm -f $tmp_file"); > } > > foreach my $config_file (@config_files) { > - $config_file =~ s/\R*//g; > + chomp $config_file; > $page_offset = parse_kernel_config_file($config_file); > if ($page_offset ne "") { > - return $page_offset; > + return hex($page_offset); > } > } > > printf STDERR "\nFailed to parse kernel config files\n"; > printf STDERR "*** NOTE ***\n"; > - printf STDERR "Falling back to PAGE_OFFSET = %s\n\n", $default_offset; > + printf STDERR "Falling back to PAGE_OFFSET = 0x%x\n\n", $default_offset; Better use the '#' flag with the 'x' conversion specifier: perl -e 'my $default_offset = hex("0xc0000000");printf "%#x\n", $default_offset' 0xc0000000 > > return $default_offset; > } > -- > 2.14.3 > > Thanks, > Kaiwan. > >> thanks, >> Tobin.
Sure, thanks Alexander.. Tobin, request you to pl make the change while merging, thanks.. Thanks & Regards, Kaiwan. On Mon, Dec 4, 2017 at 6:07 PM, Alexander Kapshuk < alexander.kapshuk@gmail.com> wrote: > On Mon, Dec 4, 2017 at 12:20 PM, <kaiwan.billimoria@gmail.com> wrote: > > On Mon, 2017-12-04 at 19:21 +1100, Tobin C. Harding wrote: > >> On Mon, Dec 04, 2017 at 10:51:53AM +0530, Kaiwan N Billimoria wrote: > >> > > --- > >> > diff --git a/scripts/leaking_addresses.pl b/scripts/ > leaking_addresses.pl > >> > index 9906dcf8b807..260b52e456f1 100755 > >> > --- a/scripts/leaking_addresses.pl > >> > +++ b/scripts/leaking_addresses.pl > >> > @@ -266,7 +266,7 @@ sub is_false_positive > >> > sub is_false_positive_ix86_32 > >> > { > >> > my ($match) = @_; > >> > - state $page_offset = eval get_page_offset(); # only gets > called once > >> > + state $page_offset = hex get_page_offset(); # only gets > called once > >> > >> I don't think this is valid ;) I meant use hex() to convert the string > >> to an int so it doesn't throw the warning (inside get_page_offset()). > > > > Yup, got it, thanks :-p > > Combined patch below: > > > > > > --- > > scripts/leaking_addresses.pl | 13 ++++++------- > > 1 file changed, 6 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl > > index 9906dcf8b807..a595a2c66b12 100755 > > --- a/scripts/leaking_addresses.pl > > +++ b/scripts/leaking_addresses.pl > > @@ -266,8 +266,7 @@ sub is_false_positive > > sub is_false_positive_ix86_32 > > { > > my ($match) = @_; > > - state $page_offset = eval get_page_offset(); # only gets called > once > > - > > + state $page_offset = get_page_offset(); # only gets called once > > if ($match =~ '\b(0x)?(f|F){8}\b') { > > return 1; > > } > > @@ -283,7 +282,7 @@ sub is_false_positive_ix86_32 > > sub get_page_offset > > { > > my $page_offset; > > - my $default_offset = "0xc0000000"; > > + my $default_offset = hex("0xc0000000"); > > my @config_files; > > > > # Allow --page-offset-32bit to override. > > @@ -306,23 +305,23 @@ sub get_page_offset > > } else { > > $page_offset = parse_kernel_config_file($tmp_ > file); > > if ($page_offset ne "") { > > - return $page_offset; > > + return hex($page_offset); > > } > > } > > system("rm -f $tmp_file"); > > } > > > > foreach my $config_file (@config_files) { > > - $config_file =~ s/\R*//g; > > + chomp $config_file; > > $page_offset = parse_kernel_config_file($config_file); > > if ($page_offset ne "") { > > - return $page_offset; > > + return hex($page_offset); > > } > > } > > > > printf STDERR "\nFailed to parse kernel config files\n"; > > printf STDERR "*** NOTE ***\n"; > > - printf STDERR "Falling back to PAGE_OFFSET = %s\n\n", > $default_offset; > > + printf STDERR "Falling back to PAGE_OFFSET = 0x%x\n\n", > $default_offset; > > Better use the '#' flag with the 'x' conversion specifier: > perl -e 'my $default_offset = hex("0xc0000000");printf "%#x\n", > $default_offset' > 0xc0000000 > > > > > return $default_offset; > > } > > -- > > 2.14.3 > > > > Thanks, > > Kaiwan. > > > >> thanks, > >> Tobin. >
Sure, thanks Alexander.. Tobin, request you to pl make the change while merging, thanks.. Thanks & Regards, Kaiwan. On Mon, Dec 4, 2017 at 6:07 PM, Alexander Kapshuk <alexander.kapshuk@gmail.com> wrote: > On Mon, Dec 4, 2017 at 12:20 PM, <kaiwan.billimoria@gmail.com> wrote: >> On Mon, 2017-12-04 at 19:21 +1100, Tobin C. Harding wrote: >>> On Mon, Dec 04, 2017 at 10:51:53AM +0530, Kaiwan N Billimoria wrote: >>> > > --- >>> > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl >>> > index 9906dcf8b807..260b52e456f1 100755 >>> > --- a/scripts/leaking_addresses.pl >>> > +++ b/scripts/leaking_addresses.pl >>> > @@ -266,7 +266,7 @@ sub is_false_positive >>> > sub is_false_positive_ix86_32 >>> > { >>> > my ($match) = @_; >>> > - state $page_offset = eval get_page_offset(); # only gets called once >>> > + state $page_offset = hex get_page_offset(); # only gets called once >>> >>> I don't think this is valid ;) I meant use hex() to convert the string >>> to an int so it doesn't throw the warning (inside get_page_offset()). >> >> Yup, got it, thanks :-p >> Combined patch below: >> >> >> --- >> scripts/leaking_addresses.pl | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl >> index 9906dcf8b807..a595a2c66b12 100755 >> --- a/scripts/leaking_addresses.pl >> +++ b/scripts/leaking_addresses.pl >> @@ -266,8 +266,7 @@ sub is_false_positive >> sub is_false_positive_ix86_32 >> { >> my ($match) = @_; >> - state $page_offset = eval get_page_offset(); # only gets called once >> - >> + state $page_offset = get_page_offset(); # only gets called once >> if ($match =~ '\b(0x)?(f|F){8}\b') { >> return 1; >> } >> @@ -283,7 +282,7 @@ sub is_false_positive_ix86_32 >> sub get_page_offset >> { >> my $page_offset; >> - my $default_offset = "0xc0000000"; >> + my $default_offset = hex("0xc0000000"); >> my @config_files; >> >> # Allow --page-offset-32bit to override. >> @@ -306,23 +305,23 @@ sub get_page_offset >> } else { >> $page_offset = parse_kernel_config_file($tmp_file); >> if ($page_offset ne "") { >> - return $page_offset; >> + return hex($page_offset); >> } >> } >> system("rm -f $tmp_file"); >> } >> >> foreach my $config_file (@config_files) { >> - $config_file =~ s/\R*//g; >> + chomp $config_file; >> $page_offset = parse_kernel_config_file($config_file); >> if ($page_offset ne "") { >> - return $page_offset; >> + return hex($page_offset); >> } >> } >> >> printf STDERR "\nFailed to parse kernel config files\n"; >> printf STDERR "*** NOTE ***\n"; >> - printf STDERR "Falling back to PAGE_OFFSET = %s\n\n", $default_offset; >> + printf STDERR "Falling back to PAGE_OFFSET = 0x%x\n\n", $default_offset; > > Better use the '#' flag with the 'x' conversion specifier: > perl -e 'my $default_offset = hex("0xc0000000");printf "%#x\n", $default_offset' > 0xc0000000 > >> >> return $default_offset; >> } >> -- >> 2.14.3 >> >> Thanks, >> Kaiwan. >> >>> thanks, >>> Tobin.
On Mon, Dec 04, 2017 at 03:50:41PM +0530, kaiwan.billimoria@gmail.com wrote: > On Mon, 2017-12-04 at 19:21 +1100, Tobin C. Harding wrote: > > On Mon, Dec 04, 2017 at 10:51:53AM +0530, Kaiwan N Billimoria wrote: > > > > --- > > > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl > > > index 9906dcf8b807..260b52e456f1 100755 > > > --- a/scripts/leaking_addresses.pl > > > +++ b/scripts/leaking_addresses.pl > > > @@ -266,7 +266,7 @@ sub is_false_positive > > > sub is_false_positive_ix86_32 > > > { > > > my ($match) = @_; > > > - state $page_offset = eval get_page_offset(); # only gets called once > > > + state $page_offset = hex get_page_offset(); # only gets called once > > > > I don't think this is valid ;) I meant use hex() to convert the string > > to an int so it doesn't throw the warning (inside get_page_offset()). > > Yup, got it, thanks :-p > Combined patch below: > > > --- > scripts/leaking_addresses.pl | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl > index 9906dcf8b807..a595a2c66b12 100755 > --- a/scripts/leaking_addresses.pl > +++ b/scripts/leaking_addresses.pl > @@ -266,8 +266,7 @@ sub is_false_positive > sub is_false_positive_ix86_32 > { > my ($match) = @_; > - state $page_offset = eval get_page_offset(); # only gets called once > - > + state $page_offset = get_page_offset(); # only gets called once > if ($match =~ '\b(0x)?(f|F){8}\b') { > return 1; > } > @@ -283,7 +282,7 @@ sub is_false_positive_ix86_32 > sub get_page_offset > { > my $page_offset; > - my $default_offset = "0xc0000000"; > + my $default_offset = hex("0xc0000000"); This is not what I meant. When you put together the whole patch just do what ever you have to do to make sure none of the functions emit warnings. My point wast that there is no need to use 'eval' to suppress warnings. I'm getting a bit lost with all these small patches in each email. Can you put together a patch with all the changes to date that you are making including the suggestions by Alexanda so we can all see where we are up to? FYI, it should apply cleanly on top of the 'leaks' branch of my tree. thanks, Tobin
diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl index 9906dcf8b807..a595a2c66b12 100755 --- a/scripts/leaking_addresses.pl +++ b/scripts/leaking_addresses.pl @@ -266,8 +266,7 @@ sub is_false_positive sub is_false_positive_ix86_32 { my ($match) = @_; - state $page_offset = eval get_page_offset(); # only gets called once - + state $page_offset = get_page_offset(); # only gets called once if ($match =~ '\b(0x)?(f|F){8}\b') { return 1; } @@ -283,7 +282,7 @@ sub is_false_positive_ix86_32 sub get_page_offset { my $page_offset; - my $default_offset = "0xc0000000"; + my $default_offset = hex("0xc0000000"); my @config_files; # Allow --page-offset-32bit to override. @@ -306,23 +305,23 @@ sub get_page_offset } else { $page_offset = parse_kernel_config_file($tmp_file); if ($page_offset ne "") { - return $page_offset; + return hex($page_offset); } } system("rm -f $tmp_file"); } foreach my $config_file (@config_files) { - $config_file =~ s/\R*//g; + chomp $config_file; $page_offset = parse_kernel_config_file($config_file); if ($page_offset ne "") { - return $page_offset; + return hex($page_offset); } } printf STDERR "\nFailed to parse kernel config files\n"; printf STDERR "*** NOTE ***\n"; - printf STDERR "Falling back to PAGE_OFFSET = %s\n\n", $default_offset; + printf STDERR "Falling back to PAGE_OFFSET = 0x%x\n\n", $default_offset; return $default_offset; }