[RFA,06/14] Remove cleanup_iconv
Commit Message
This introduces a new "iconv_wrapper" class, to be used in
convert_between_encodings. This allows the removal of cleanup_iconv.
2017-04-07 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 | 44 ++++++++++++++++++++++++++++----------------
2 files changed, 34 insertions(+), 16 deletions(-)
Comments
On 2017-04-08 16:12, Tom Tromey wrote:
> --- a/gdb/charset.c
> +++ b/gdb/charset.c
> @@ -481,14 +481,33 @@ 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 ()
> + {
> + if (m_desc != (iconv_t) -1)
> + iconv_close (m_desc);
From what I understand about C++ constructors and destructors, the check
for -1 is unnecessary. The destructor will only be called if the
constructor ran to completion. If the constructor ran to completion
(didn't throw), m_desc won't be -1.
Otherwise, LGTM.
Thanks,
Simon
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
Simon> From what I understand about C++ constructors and destructors, the
Simon> check for -1 is unnecessary. The destructor will only be called if
Simon> the constructor ran to completion. If the constructor ran to
Simon> completion (didn't throw), m_desc won't be -1.
Yeah, that was just copy-paste from the other class, which may be wrong.
I changed this.
Tom
@@ -1,5 +1,11 @@
2017-04-07 Tom Tromey <tom@tromey.com>
+ * charset.c (iconv_wrapper): New class.
+ (cleanup_iconv): Remove.
+ (convert_between_encodings): Use it.
+
+2017-04-07 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>.
@@ -481,14 +481,33 @@ 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 ()
+ {
+ if (m_desc != (iconv_t) -1)
+ 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 +515,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 +526,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 +545,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 +597,6 @@ convert_between_encodings (const char *from, const char *to,
}
}
}
-
- do_cleanups (cleanups);
}