From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 91104 invoked by alias); 28 Mar 2015 16:56:21 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 91092 invoked by uid 89); 28 Mar 2015 16:56:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 X-HELO: homiemail-a115.g.dreamhost.com Received: from sub5.mail.dreamhost.com (HELO homiemail-a115.g.dreamhost.com) (208.113.200.129) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 28 Mar 2015 16:56:19 +0000 Received: from homiemail-a115.g.dreamhost.com (localhost [127.0.0.1]) by homiemail-a115.g.dreamhost.com (Postfix) with ESMTP id EC15D570E; Sat, 28 Mar 2015 09:56:17 -0700 (PDT) Received: from redwood.eagercon.com (c-24-7-16-38.hsd1.ca.comcast.net [24.7.16.38]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: eager@eagerm.com) by homiemail-a115.g.dreamhost.com (Postfix) with ESMTPSA id C7248570D; Sat, 28 Mar 2015 09:56:17 -0700 (PDT) Message-ID: <5516DD31.4020901@eagerm.com> Date: Sat, 28 Mar 2015 16:56:00 -0000 From: Michael Eager User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 MIME-Version: 1.0 To: "gdb-patches@sourceware.org" CC: Pedro Alves , Doug Evans Subject: Re: [PATCH] Support gzip compressed exec and core files in gdb References: <54FF77D6.7010400@eagerm.com> <550A1F4D.5030107@eagerm.com> <20150320221640.GR11803@vapier> <551576A0.3060504@eagerm.com> In-Reply-To: <551576A0.3060504@eagerm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00947.txt.bz2 On 03/27/15 08:26, Michael Eager wrote: > Removed binutils@sourceware.org. > > On 03/20/15 15:16, Mike Frysinger wrote: >> On 18 Mar 2015 17:58, Michael Eager wrote: >>> --- a/gdb/utils.c >>> +++ b/gdb/utils.c >>> >>> +#define COMPRESS_BUF_SIZE (1024*1024) >> >> space around the * >> >> would be nice to have a comment noting where the 1MiB comes from > > I added a comment, but the buffer size is arbitrary. > >>> + /* Create temporary file name for uncompressed file. */ >>> + if (!(tmpdir = getenv ("TMPDIR"))) >>> + tmpdir = "/tmp"; >>> + >>> + if (!asprintf (&template, "%s/%s-XXXXXX", tmpdir, basename (filename))) >>> + return 0; >>> + >>> + decomp_fd = mkstemp (template); >> >> ignoring that assigningments in if statements are frowned upon, looks like you >> can just use make_temp_file(NULL) from libiberty > > Fixed assignment. make_temp_file() generates an anonymous file; I want to > keep the user-specified file name as part of the uncompressed file name. > (I could update libiberty to add an alternate to make_temp_file() which > takes a basename, I guess.) I replaced getenv("TMPDIR") with choose_tmpdir() > from libiberty. > >> you would also leak the fopen() handle here. probably want to go through this >> code again looking for such leaks. and maybe try breaking the func up into more >> utility related chunks when possible. > > I refactored the gdb_uncompress() function and broke it into 3 functions. > >> you've got more missing tabs in this file. i'm going to stop checking. > > I cleaned up the white space. I hope I got all the tabs this time. > > gdb/ChangeLog: > * utils.c (struct compressed_file_cache_search, eq_compressed_file, > is_gzip, decompress_gzip, do_compressed_cleanup, identify_compression, > uncompress_to_temporary, gdb_uncompress): New. > * utils.h (gdb_uncompress): Declare. > * corelow.c (core_open): Uncompress core file. > * exec.c (exec_file_attach): Uncompress exec file. > * symfile.c (symfile_bfd_open): Uncompress sym (exec) file. > * NEWS: Mention new functionality. > > gdb/doc: > * gdb.texinfo (Files): Mention gzipped exec and core files. > > > I'm not fond of whitespace patches, but there were many whitespace errors > in these files unrelated to my patch. I've attached a second patch to clean > this up. > > gdb/ChangeLog: > * corelow.c: Replace leading whitespace with tabs, eliminate trailing blanks. > * exec.c: Ditto. > * symfile.c: Ditto. > * utils.c: Ditto > * utils.h: Ditto Pedro, Doug -- Can you approve this patch? -- Michael Eager eager@eagercon.com 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077