Re: [bug-patch] [PATCH] avoid using stat.st_dev and st_ino members of uninitialized "outst"

[Jim Meyering]

Andreas Gruenbacher wrote:
> Hi Jim,
>
> On Monday 07 February 2011 19:17:02 Jim Meyering wrote:
>> Testing my recent changes, I found an unrelated used-uninitialized bug.
>> It did not look easy to abuse, since the affected data are used only in
>> determining whether a file has already been processed.
>
> I see how this can trigger for ed-style diffs because outst is not used.
> That can easily be fixed though (plus safety check):
>
>
> diff --git a/src/patch.c b/src/patch.c
> index 77be499..0cdaeb9 100644
> --- a/src/patch.c
> +++ b/src/patch.c
> @@ -105,6 +105,7 @@ main (int argc, char **argv)
>      mode_t file_type;
>      int outfd = -1;
>
> +    outst.st_size = -1;
>      exit_failure = 2;
>      program_name = argv[0];
>      init_time ();
> @@ -248,10 +249,9 @@ main (int argc, char **argv)
>                     outstate.ofp);
>       if (! dry_run && ! outfile && ! skip_rest_of_patch)
>         {
> -         struct stat statbuf;
> -         if (fstat (outfd, &statbuf) != 0)
> +         if (fstat (outfd, &outst) != 0)
>             pfatal ("%s", TMPOUTNAME);
> -         outstate.zero_output = statbuf.st_size == 0;
> +         outstate.zero_output = outst.st_size == 0;
>         }
>       close (outfd);
>       outfd = -1;
> @@ -500,6 +500,7 @@ main (int argc, char **argv)
>                       set_file_attributes (TMPOUTNAME, attr, inname, &instat,
>                                            mode, &new_time);
>
> +                   assert (outst.st_size != -1);
>                     move_file (TMPOUTNAME, &TMPOUTNAME_needs_removal, &outst,
>                                outname, mode, backup);

That patch is definitely smaller than mine.

> Do you see a way how the assert could still trigger?

No.  I prefer yours, even if it's a little harder to review.
Good work.