From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120147 invoked by alias); 20 Mar 2015 22:16:44 -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 120125 invoked by uid 89); 20 Mar 2015 22:16:43 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: smtp.gentoo.org Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Fri, 20 Mar 2015 22:16:43 +0000 Received: from vapier (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with SMTP id 75C23340C10; Fri, 20 Mar 2015 22:16:40 +0000 (UTC) Date: Fri, 20 Mar 2015 22:16:00 -0000 From: Mike Frysinger To: Michael Eager Cc: "gdb-patches@sourceware.org" , binutils Subject: Re: [PATCH] Support gzip compressed exec and core files in gdb Message-ID: <20150320221640.GR11803@vapier> Mail-Followup-To: Michael Eager , "gdb-patches@sourceware.org" , binutils References: <54FF77D6.7010400@eagerm.com> <550A1F4D.5030107@eagerm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="o/5eNASeIIpuMggS" Content-Disposition: inline In-Reply-To: <550A1F4D.5030107@eagerm.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00679.txt.bz2 --o/5eNASeIIpuMggS Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-length: 2768 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 > +static int > +decompress_gzip (const char *filename, FILE *tmp) > +{ > +#ifdef HAVE_ZLIB_H > + char *buf =3D xmalloc (COMPRESS_BUF_SIZE); > + gzFile compressed =3D gzopen (filename, "r"); > + int count, res; > + > + if (buf =3D=3D NULL || compressed =3D=3D NULL) > + { > + fprintf_filtered (gdb_stderr, _("error copying gzip file\n")); > + free (buf); > + return 0; > + } > + > + while ((count =3D gzread (compressed, buf, COMPRESS_BUF_SIZE))) > + { > + res =3D fwrite (buf, 1, count, tmp); > + if (res !=3D count) > + { > + fprintf_filtered (gdb_stderr, _("error decompressing gzip file\n")); > + free (buf); this line needs to use a tab to indent > +/* If file is compressed, uncompress it into a temporary. */ > + > +int > +gdb_uncompress (const char *filename, char **uncompressed_filename) > +{ > + FILE *handle; > + struct compressed_file_cache_search search, *found; > + struct stat st; > + hashval_t hash; > + void **slot; > + static unsigned char buffer[1024]; > + size_t count; > + enum {NONE, GZIP, BZIP2} file_compression =3D NONE; shouldn't this enum be declared outside this func ? > + if (found) > + { > + /* We previously uncompressed the file. */ > + if (found->mtime =3D=3D st.st_mtime) > + { this brace needs to indent w/a tab > + /* Return file if compressed file not changed. */ > + *uncompressed_filename =3D found->uncompressed_filename; > + return 1; > + } > + else > + { same here > + /* Delete old uncompressed file. */ > + unlink (found->uncompressed_filename); > + xfree (found->filename); > + xfree (found->uncompressed_filename); > + } and here > + /* Create temporary file name for uncompressed file. */ > + if (!(tmpdir =3D getenv ("TMPDIR"))) > + tmpdir =3D "/tmp"; > + > + if (!asprintf (&template, "%s/%s-XXXXXX", tmpdir, basename (filename))) > + return 0; > + > + decomp_fd =3D mkstemp (template); ignoring that assigningments in if statements are frowned upon, looks like = you=20 can just use make_temp_file(NULL) from libiberty you would also leak the fopen() handle here. probably want to go through t= his=20 code again looking for such leaks. and maybe try breaking the func up into= more=20 utility related chunks when possible. > + if (decomp_fd !=3D -1) > + { > + decomp_file =3D fdopen (decomp_fd, "w+b"); FOPEN_WUB > + if (file_compression =3D=3D GZIP) > + { you've got more missing tabs in this file. i'm going to stop checking. -mike --o/5eNASeIIpuMggS Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-length: 819 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJVDJxIAAoJEEFjO5/oN/WBOM0P/3J9y7pdXFGUma3Nw9obdEbV 5n8CVQssNj/LCVn+YFI8PsyRFzSG8uCHyeVg3y9aUz55/SXJ0qNkkNQZ5mXJhdat MO+g9iNoteyY1QH9Ae8vhBBoubWiowhiJygL4F+vbPPkFvO6M2+6LdiJ4yu7eLKy 5VLhKNeMtDlNTFcLdveS0a7CtL+I78J11oqRH4rzLIsTAgDFJ8yi3SPmDhkBezvZ si2Uf3qPzefFS8YAx4dd2UdRGrkz9KRQpPXe8xDo4apvXTdKsZhP/+4DgRxLmgOy 3i3xFZPMdIW8a5J6EukcbIquSxRfopnbK8+/khdOkrl5+To2MrYM0oQeEJYlg6I1 qzLB94MxuoNjuyHMchHDbbcwKgALLR/ez4U+8OPsRMtU4d4GXfcFUzgqmFQrBdnp JvuONhQvDeI4rfB3BEqRjtva53lKI5ZKGxUPEEkUZWOG3YcP7d/ENuWD3neu2H18 kltJrUykp/x0ucOo/Uw7RTqBYqGZ9GQLpVD234oOnTWN3gfMsjcEVmKL6cqL7ceM GgEQKZf/2ySsvgOlW0CKWEHHqYYdETwglat207cRYFevRW7+x5+eA2CI6yHXhAlK la7PUtTs3FYH/4HsA3x18FLIvgD7JxYLku+kPxZV8KzZbOgzTG4Sj47jIWpvSB/6 mn3yRAaObVNxurMxjM2b =MWJk -----END PGP SIGNATURE----- --o/5eNASeIIpuMggS--