[PATCH][PING] Keep patch file permissions in mklog

Yury Gribov y.gribov@samsung.com
Thu Sep 18 14:56:00 GMT 2014


On 08/04/2014 12:14 PM, Tom de Vries wrote:
> On 04-08-14 08:45, Yury Gribov wrote:
>> Thanks! My 2 (actually 4) cents below.
>>
>
> Hi Yuri,
>
> thanks for the review.
>
>>  > +if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq
>> "--inline")) {
>>  > +    $diff = $ARGV[1];
>>
>> Can we shift here and then just set $diff to $ARGV[0] unconditionally?
>>
>
> Done.
>
>>  > +    if ($diff eq "-") {
>>  > +            die "Reading from - and using -i are not compatible";
>>  > +    }
>>
>> Hm, can't we dump ChangeLog to stdout in that case?
>> The limitation looks rather strange.
>>
>
> My original idea here was that --inline means 'in the patch file', which
> is not possible if the patch comes from stdin.
>
> I've now interpreted it such that --inline prints to stdout what it
> would print to the patch file otherwise, that is, both log and patch.
> Printing just the log to stdout can be already be achieved by not using
> --inline.
>
>>  > +    open (FILE1, '>', $tmp) or die "Could not open temp file";
>>
>> Could we use more descriptive name?
>>
>
> I've used the slightly more descriptive 'OUTPUTFILE'.
>
>>  > +    system ("cat $diff >>$tmp") == 0
>>  > +        or die "Could not append patch to temp file";
>>  > ...
>>  > +    unlink ($tmp) == 1 or die "Could not remove temp file";
>>
>> The checks look like an overkill given that we don't check for result
>> of mktemp...
>>
>
> I've added a check for the result of mktemp, and removed the unlink
> result check. I've left in the "Could not append patch to temp file"
> check because the patch file might be read-only.
>
> OK for trunk?
>
> Thanks,
> - Tom
>

Pinging the patch for Tom.



More information about the Gcc-patches mailing list