[PATCH] Keep patch file permissions in mklog

[Tom de Vries]

On 04-08-14 13:50, Yury Gribov wrote:
>  > thanks for the review.
>
> Np, I'm personally happy to see that script is useful.
>
>  > 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.
>
> Could you add a note in the help message?
>

Done.

> +if ($#ARGV == 1 && ("$ARGV[0]" eq "-i" || "$ARGV[0]" eq "--inline")) {
>
> I'd do >= 1 but that's a question of personal preference.
>

What is the purpose of that proposed change ?

> +if ($inline && $diff ne "-") {
> +    $tmp = `mktemp`;
> +    if ($? != 0) {
> +        die "Could not generate temp file";
> +    }
>
> IMHO better use consistent style: system() or ticks with $?.
> Or even better, encapsulate environment calls in a subfunction which would check
> return code and return stdout.
>

I've realised that using cat to keep permissions on the patch file might not be 
a good idea: if it's interrupted, the original patch might be truncated. So I 
use mv at the end to atomically move the new contents to the patch file. And I 
implement keeping the permission by first copying the patch file to the temp 
file. That means that it's necessary to first truncate the temp file before 
writing to it. I've implemented the truncate using open.

I've tried using a temp file with just a file handle, as Segher suggested, (and 
use perl truncate to do the truncation), but I didn't get that working. So I'm 
now using mktemp from File::Temp.

With these modification, I've eliminated system() calls and backticks from the 
patch.

> BTW you may want to wait for Diego's and Trevor's comments (Diego is the
> maintainer and approver for this code).
>

ok.

Thanks,
- Tom

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-inline-option-to-contrib-mklog.patch
Type: text/x-patch
Size: 2472 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20140811/b1aaef31/attachment.bin>