From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48576 invoked by alias); 11 Mar 2015 02:37:59 -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 48531 invoked by uid 89); 11 Mar 2015 02:37:58 -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; Wed, 11 Mar 2015 02:37:57 +0000 Received: from vapier (localhost [127.0.0.1]) by smtp.gentoo.org (Postfix) with SMTP id 3C2E733BDE7; Wed, 11 Mar 2015 02:37:55 +0000 (UTC) Date: Wed, 11 Mar 2015 02:37: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: <20150311023755.GX9455@vapier> Mail-Followup-To: Michael Eager , "gdb-patches@sourceware.org" , binutils References: <54FF77D6.7010400@eagerm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="CRrRoVXEpX/Dc4YP" Content-Disposition: inline In-Reply-To: <54FF77D6.7010400@eagerm.com> X-IsSubscribed: yes X-SW-Source: 2015-03/txt/msg00269.txt.bz2 --CRrRoVXEpX/Dc4YP Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-length: 2729 On 10 Mar 2015 16:01, Michael Eager wrote: > --- a/gdb/common/filestuff.c > +++ b/gdb/common/filestuff.c > > +#define COMPRESS_BUF_SIZE (1024*1024) > +static int > +decompress_gzip (const char *filename, FILE *tmp) > +{ > +#ifdef HAVE_ZLIB_H > + char *buf =3D malloc (COMPRESS_BUF_SIZE); xmalloc ? > + if (buf =3D=3D NULL || compressed =3D=3D NULL) > + { > + printf (_("error copying gzip file\n")); fprintf_filtered (gdb_stderr, ...) ? > + printf (_("error decompressing gzip file\n")); here too ? > + free (buf); indentation is broken. this comes up a lot, so you should scan the whole t= hing. > + fflush (tmp); that needed ? > +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; > + int decomp_fd; > + FILE *decomp_file; > + int ret =3D 0; > + char *tmpdir, *p; > + char *template =3D xmalloc(128 + 12 + 7 + 1); probably should be a comment as to the constants you've written here > + if (compressed_file_cache =3D=3D NULL) style says there should be a blank line above this if statement > + compressed_file_cache =3D htab_create_alloc (1, htab_hash_string, > + eq_compressed_file, > + NULL, xcalloc, xfree); > + > + if ((stat (filename, &st) < 0)) excess set of paren > + /* Return file if compressed file not changed. */ > + *uncompressed_filename =3D found->uncompressed_filename; > + return 1; > + } > + else > + { > + /* Delete old uncompressed file. */ > + unlink (found->uncompressed_filename); > + xfree ((void *)found->filename); is that cast really needed ? > + if ((handle =3D fopen (filename, "rb")) =3D=3D NULL) gdb generally doesn't like to pack assignments into if statements use FOPEN_RB instead of "rb" ? > + /* Create temporary file name for uncompressed file. */ > + if (!(tmpdir =3D getenv ("TMPDIR"))) > + tmpdir =3D "/tmp"; > + strncpy (template, tmpdir, 128); > + strcat (template, "/"); > + for (p =3D (char *)filename + strlen (filename) - 1; > + p >=3D filename && *p !=3D '/'; p--) /* find final slash. */ ; > + strncat (template, ++p, 128); > + p =3D template + strlen (template); > + if (strcmp (p - 3, ".gz") =3D=3D 0) > + *(p - 3) =3D '\0'; > + strcat (template, "-XXXXXX"); that's pretty messy man. why not use mkstemp() and fdopen() instead ? in general, there's no need for all this strcat business. asprintf allows = yout=20 easily concat paths & allocate the right amount of space. -mike --CRrRoVXEpX/Dc4YP Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature Content-length: 819 -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJU/6qDAAoJEEFjO5/oN/WBAFsP/3zVdjcBIj8iqimBma/CcsOS 8tfyLSFrKIYuynz3DoT+4w1j0QPUBPjVN+Uism8N5nod/LklHVolWrn2EGp++HDr xRxqgqsTacz9KpXv8JqlRJrIndlY2q5rt5hWf+uTXDgyNqhMz9akPYFAgReRB1w4 mDgxSTEYOXh6jBQKKMO8+ITGeNooNsXpV/T6GP41YNDXgVun/v9KWd1IxrKrgdlv 9GvEUVVcPZGCk7o7bylbrkcixTvmZW4m2x8EGgbFuuHQBootbliDxe3LEFBnx3zD 4Mg9rlnLMPclMNHDBkPxB6yCmbMb6TSwcon3bdglBv9WIry7CCMcFxA8bqF3xZFf KCqq0nRQiDFksS1KMAwt0hM0325WoGuF3UWEIXWFE660VXGFdEtcdYJ4n4Atrw8s fC76L2QKSmhmL1JWDKySz+W/awko2v5cAMGlmRD9ILuN8tg9WqCoJ8UsX96D0JOm Kmwpq6kg0VYMhI87AAc8g55JMvO/BnDLrEqflgv4EC372Ci1fqb82j3NTBktP6FJ hMUYv3lzu5+H2XjTCFSjdi9u6bXXsN4atLbcVuA3J6Ik6XA0+aTwe+YngLxodg3v OVsKADkod74gb/D77PoG3vOKKwQpqjynWTUPGwnSndKPpPyAsZicpP+0jEWU2z4v jDzBEYOG44O/WeIgXh5S =zd1+ -----END PGP SIGNATURE----- --CRrRoVXEpX/Dc4YP--