diff mbox

[RFA,v2,06/17] Remove cleanup_iconv

Message ID 20170411150112.23207-7-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey April 11, 2017, 3:01 p.m. UTC 
  This introduces a new "iconv_wrapper" class, to be used in
convert_between_encodings.  This allows the removal of cleanup_iconv.

gdb/ChangeLog
2017-04-11  Tom Tromey  <tom@tromey.com>

	* charset.c (iconv_wrapper): New class.
	(cleanup_iconv): Remove.
	(convert_between_encodings): Use it.
---
 gdb/ChangeLog |  6 ++++++
 gdb/charset.c | 43 +++++++++++++++++++++++++++----------------
 2 files changed, 33 insertions(+), 16 deletions(-)

Comments

Pedro Alves April 12, 2017, 11:19 a.m. UTC | #1 
On 04/11/2017 04:01 PM, Tom Tromey wrote:
> +  iconv_wrapper (const char *from, const char *to)
> +  {
> +    m_desc = iconv_open (to, from);

...

> -  desc = iconv_open (to, from);
> -  if (desc == (iconv_t) -1)
> -    perror_with_name (_("Converting character sets"));
> -  cleanups = make_cleanup (cleanup_iconv, &desc);
> +  iconv_wrapper desc (from, to);
>  

I find the inversion of the order of "from" and "to" potentially
confusing/misleading, given this is a "wrapper" class.  Is there a
rationale behind the swap?

Thanks,
Pedro Alves
Tom Tromey April 12, 2017, 2:32 p.m. UTC | #2 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> I find the inversion of the order of "from" and "to" potentially
Pedro> confusing/misleading, given this is a "wrapper" class.  Is there a
Pedro> rationale behind the swap?

It mirrors the call to convert_between_encodings, but I agree with your
critique, so I've changed it.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1c76edb..c50eaa8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@ 
 2017-04-11  Tom Tromey  <tom@tromey.com>
 
+	* charset.c (iconv_wrapper): New class.
+	(cleanup_iconv): Remove.
+	(convert_between_encodings): Use it.
+
+2017-04-11  Tom Tromey  <tom@tromey.com>
+
 	* symfile.h (increment_reading_symtab): Update type.
 	* symfile.c (decrement_reading_symtab): Remove.
 	(increment_reading_symtab): Return a scoped_restore_tmpl<int>.
diff --git a/gdb/charset.c b/gdb/charset.c
index 8302c59..a6da512 100644
--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -481,14 +481,32 @@  host_hex_value (char c)
 
 /* Public character management functions.  */
 
-/* A cleanup function which is run to close an iconv descriptor.  */
-
-static void
-cleanup_iconv (void *p)
+class iconv_wrapper
 {
-  iconv_t *descp = (iconv_t *) p;
-  iconv_close (*descp);
-}
+public:
+
+  iconv_wrapper (const char *from, const char *to)
+  {
+    m_desc = iconv_open (to, from);
+    if (m_desc == (iconv_t) -1)
+      perror_with_name (_("Converting character sets"));
+  }
+
+  ~iconv_wrapper ()
+  {
+    iconv_close (m_desc);
+  }
+
+  size_t convert (ICONV_CONST char **inp, size_t *inleft, char **outp,
+		  size_t *outleft)
+  {
+    return iconv (m_desc, inp, inleft, outp, outleft);
+  }
+
+private:
+
+  iconv_t m_desc;
+};
 
 void
 convert_between_encodings (const char *from, const char *to,
@@ -496,8 +514,6 @@  convert_between_encodings (const char *from, const char *to,
 			   int width, struct obstack *output,
 			   enum transliterations translit)
 {
-  iconv_t desc;
-  struct cleanup *cleanups;
   size_t inleft;
   ICONV_CONST char *inp;
   unsigned int space_request;
@@ -509,10 +525,7 @@  convert_between_encodings (const char *from, const char *to,
       return;
     }
 
-  desc = iconv_open (to, from);
-  if (desc == (iconv_t) -1)
-    perror_with_name (_("Converting character sets"));
-  cleanups = make_cleanup (cleanup_iconv, &desc);
+  iconv_wrapper desc (from, to);
 
   inleft = num_bytes;
   inp = (ICONV_CONST char *) bytes;
@@ -531,7 +544,7 @@  convert_between_encodings (const char *from, const char *to,
       outp = (char *) obstack_base (output) + old_size;
       outleft = space_request;
 
-      r = iconv (desc, &inp, &inleft, &outp, &outleft);
+      r = desc.convert (&inp, &inleft, &outp, &outleft);
 
       /* Now make sure that the object on the obstack only includes
 	 bytes we have converted.  */
@@ -583,8 +596,6 @@  convert_between_encodings (const char *from, const char *to,
 	    }
 	}
     }
-
-  do_cleanups (cleanups);
 }