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