From owner-p4-projects@FreeBSD.ORG Tue Apr 13 04:36:13 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 452341065676; Tue, 13 Apr 2010 04:36:13 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 043D81065670 for ; Tue, 13 Apr 2010 04:36:13 +0000 (UTC) (envelope-from kientzle@freebsd.org) Received: from monday.kientzle.com (kientzle.com [66.166.149.50]) by mx1.freebsd.org (Postfix) with ESMTP id CA2B48FC13 for ; Tue, 13 Apr 2010 04:36:12 +0000 (UTC) Received: (from root@localhost) by monday.kientzle.com (8.14.3/8.14.3) id o3D3uR9k029367; Tue, 13 Apr 2010 03:56:27 GMT (envelope-from kientzle@freebsd.org) Received: from horton.x.kientzle.com (fw2.kientzle.com [10.123.1.2]) by kientzle.com with SMTP id aexbfvexqniv4v8aj4wwe42iqs; Tue, 13 Apr 2010 03:56:27 +0000 (UTC) (envelope-from kientzle@freebsd.org) Message-ID: <4BC3EB5B.5070801@freebsd.org> Date: Mon, 12 Apr 2010 20:56:11 -0700 From: Tim Kientzle User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.8.1.23) Gecko/20100314 SeaMonkey/1.1.18 MIME-Version: 1.0 To: Garrett Cooper References: <201004121230.o3CCUsIX029146@repoman.freebsd.org> In-Reply-To: <201004121230.o3CCUsIX029146@repoman.freebsd.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Perforce Change Reviews Subject: Re: PERFORCE change 176831 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Apr 2010 04:36:13 -0000 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.)