Date: Mon, 12 Apr 2010 20:56:11 -0700 From: Tim Kientzle <kientzle@freebsd.org> To: Garrett Cooper <gcooper@freebsd.org> Cc: Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 176831 for review Message-ID: <4BC3EB5B.5070801@freebsd.org> In-Reply-To: <201004121230.o3CCUsIX029146@repoman.freebsd.org> References: <201004121230.o3CCUsIX029146@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Garrett Cooper wrote: > +FILE* > +unpack_file_to_fd(const char *pkg, const char *file) > +{ .... > + r = archive_read_extract(archive, archive_entry, > + EXTRACT_ARCHIVE_FLAGS); > + if (r == ARCHIVE_OK) { > + if (Verbose) > + printf("X - %s\n", > + entry_pathname); > + fd = fopen(entry_pathname, "r"); This is potentially race-prone. I would suggest instead that you: int fd = open(entry_pathname, O_RDWR | O_CREAT, ...) archive_read_data_into_fd(a, fd); Then rewind the fd and return it. archive_read_extract() is handy but it's way overkill for this kind of simple operation. I would also put in a simple verification that the entry you found is a regular entry: archive_entry_filetype(entry) == AE_IFREG > + * NOTE: the exit code is 0 / 1 so that this can be fed directly into exit > + * when doing piped tar commands for copying hierarchies *hint*, *hint*. Why do you want to copy hierarchies? Seems a waste of disk bandwidth. *hint* *hint* ;-) > int > unpack(const char *pkg, const char *file_expr) > { Since this is only called with file_expr=="*" and file_expr=="+*", I don't think you need fnmatch(). You can get away with a simple int unpack(const char *pkg, int metadata_only) and then use if (!metadata_only || entry_pathname[0] == '+') > + (fnmatch(file_expr, entry_pathname, > + FNM_PATHNAME)) == 0) { > You might also consider passing in an fd to unpack and using archive_read_open_fd() to attach an archive handle to it instead of archive_read_open_filename(). (It doesn't make any performance difference, but it does avoid races and opens the door to other interesting games in the future.)
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4BC3EB5B.5070801>