This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Support gzip compressed exec and core files in gdb


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]