[pve-devel] [PATCH common v2 2/3] INotify.pm: use run_command instead of open for calling diff

[Dominik Csapak]

On 09/07/2018 10:08 AM, Thomas Lamprecht wrote:
> On 9/5/18 10:54 AM, Dominik Csapak wrote:
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   src/PVE/INotify.pm | 15 +++++++--------
>>   1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/PVE/INotify.pm b/src/PVE/INotify.pm
>> index f17f4c5..184f00d 100644
>> --- a/src/PVE/INotify.pm
>> +++ b/src/PVE/INotify.pm
>> @@ -19,7 +19,8 @@ use PVE::ProcFSTools;
>>   use Clone qw(clone);
>>   use Linux::Inotify2;
>>   use base 'Exporter';
>> -use JSON;
>> +use JSON;
>> +use Encode qw(encode decode);
>>   
>>   our @EXPORT_OK = qw(read_file write_file register_file);
>>   
>> @@ -56,13 +57,11 @@ sub ccache_compute_diff {
>>   
>>       my $diff = '';
>>   
>> -    open (TMP, "diff -b -N -u '$filename' '$shadow'|");
>> -	
>> -    while (my $line = <TMP>) {
>> -	$diff .= $line;
>> -    }
>> -
>> -    close (TMP);
>> +    my $cmd = ['/usr/bin/diff', '-b', '-N', '-u', $filename, $shadow];
>> +    PVE::Tools::run_command($cmd, noerr => 1, outfunc => sub {
>> +	my ($line) = @_;
>> +	$diff .= decode('UTF-8', $line) . "\n";
> 
> you also silently switch over to UTF-8 decoding all input without
> any mention in the commit message... What's that about?
> 
>> +    });
>>   
>>       $diff = undef if !$diff;
>>   
>>
> 

hmm yeah sorry,

i noticed that showing the diff of the network interfaces file with utf8 
comments had the same problem with displaying it so i inserted it there

i am not sure about the correct behaviour here

we only use the diff for /etc/network/interfaces (AFAICS)
but it does not feel right to e.g. only decode lines starting with '#'

alternatives?