[pacman-dev] [PATCH] Rework extract_single_file() temp file creation

Xavier shiningxc at gmail.com
Tue Apr 29 03:57:34 EDT 2008


On Tue, Apr 29, 2008 at 5:39 AM, Dan McGee <dan at archlinux.org> wrote:
> On Mon, Apr 28, 2008 at 10:34 PM, Dan McGee <dan at archlinux.org> wrote:
>  > We were a bit juryrigged using one call to mkstemp() before rather than
>  >  extracting the new files side-by-side and doing our comparisons there. We
>  >  were also facing some permissions issues. Instead, make our life easier by
>  >  extracting all temp files to a '.paccheck' extension, doing our md5
>  >  comparisons, and then taking the correct actions.
>  >
>  >  Still to be done here- a cleanup of the use of PATH_MAX which should not be
>  >  necessary if we use dynamic allocation on the heap.
>  >
>  >  Signed-off-by: Dan McGee <dan at archlinux.org>
>  >  ---
>
>  I posted these patches here because I would love to get some feedback
>  on them from you guys, and they need some real-life testing. The
>  biggest catch with this patch is ensuring we don't leave any paccheck
>  files behind, as we can no longer always unlink the original file
>  since we use rename instead of copy.
>
>  All of these can be found on my public 'working' branch if you are
>  tracking my GIT repository. I'll hold off a few days before pushing
>  these to master.
>

At least it builds fine on cygwin, and make check is fine too :)
And reading the code, it looks fine too.
Just about this paccheck issue, do we really need to do it that way
and have to worry about it?
Why can't we just let the unlink call? If the file is here, it will
always be removed. If its not here anymore, unlink will fail but that
doesn't matter.
Otherwise an explicit stat could be made to see if the file is still here.
But maybe you find this ugly and prefer handling everything manually,
at the risk of leaving some files behind in case we missed something?

But if we keep your patch as is, there are some failure cases where
you kept the paccheck on purpose, right?
Every time the rename call fails, there is no unlink. But I doubt this
happens often anyway, so no big problem I guess.




More information about the pacman-dev mailing list