[PATCH] Support gzip compressed exec and core files in gdb
Mike Frysinger
vapier@gentoo.org
Wed Mar 11 02:37:00 GMT 2015
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 = malloc (COMPRESS_BUF_SIZE);
xmalloc ?
> + if (buf == NULL || compressed == 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 thing.
> + 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 = NONE;
> + int decomp_fd;
> + FILE *decomp_file;
> + int ret = 0;
> + char *tmpdir, *p;
> + char *template = xmalloc(128 + 12 + 7 + 1);
probably should be a comment as to the constants you've written here
> + if (compressed_file_cache == NULL)
style says there should be a blank line above this if statement
> + compressed_file_cache = 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 = 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 = fopen (filename, "rb")) == 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 = getenv ("TMPDIR")))
> + tmpdir = "/tmp";
> + strncpy (template, tmpdir, 128);
> + strcat (template, "/");
> + for (p = (char *)filename + strlen (filename) - 1;
> + p >= filename && *p != '/'; p--) /* find final slash. */ ;
> + strncat (template, ++p, 128);
> + p = template + strlen (template);
> + if (strcmp (p - 3, ".gz") == 0)
> + *(p - 3) = '\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
easily concat paths & allocate the right amount of space.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20150311/d51b0aed/attachment.sig>
More information about the Gdb-patches
mailing list