Clean up iconv/gconv_int.h for unnecessary declarations
diff mbox series

Message ID 20171027151849.GA95692@aloka.lostca.se
State New
Headers show
Series
  • Clean up iconv/gconv_int.h for unnecessary declarations
Related show

Commit Message

Arjun Shankar Oct. 27, 2017, 3:18 p.m. UTC
The variables __gconv_path_elem, __gconv_max_path_elem_len and function
__gconv_get_path declared in, as well as the type path_elem and macro
GCONV_NCHAR_GOAL defined in gconv_int.h are all used in only one iconv
compilation unit each. In addition, the extern declaration of the variable
__gconv_nmodules refers to a variable that does not exist any more.
Considering this, these symbols do not need to be exposed via a header file.

This patch removes the extern declarations from the header file and moves
the definitions to the compilation units where they are used.

ChangeLog:

2017-10-27  Arjun Shankar  <arjun@redhat.com>

	* iconv/gconv_int.h (__gconv_path_elem): Remove.
	(__gconv_max_path_elem_len): Likewise.
	(__gconv_nmodules): Likewise.
	(__gconv_get_path): Likewise.
	(path_elem): Move to ...
	* iconv/gconv_conf.c: ... here.
	(__gconv_get_path): Mark function static.
	* iconv/gconv_int.h (GCONV_NCHAR_GOAL): Move to ...
	* iconv/gconv_open.c: ... here.

---
 iconv/gconv_conf.c |  9 ++++++++-
 iconv/gconv_int.h  | 21 ---------------------
 iconv/gconv_open.c |  4 ++++
 3 files changed, 12 insertions(+), 22 deletions(-)

Comments

Arjun Shankar Oct. 27, 2017, 3:38 p.m. UTC | #1
> This patch removes the extern declarations from the header file and moves
> the definitions to the compilation units where they are used.

One of the context lines in this patch depends on a patch I submitted
yesterday [1]. Nonetheless, it can still be applied by passing '-C2' to
git-apply or git-am. Apologies for the confusion.

I  am referring to this hunk in iconv/gconv_conf.c:

> @@ -420,7 +427,7 @@ read_conf_file (const char *filename, const char *directory, size_t dir_len,
>  
>  
>  /* Determine the directories we are looking for data in.  */
> -void
> +static void
>  __gconv_get_path (void)
>  {
>    /* Make sure we haven't done it already.  */

[1] https://sourceware.org/ml/libc-alpha/2017-10/msg01195.html
Carlos O'Donell Nov. 1, 2017, 5:06 a.m. UTC | #2
On 10/27/2017 08:18 AM, Arjun Shankar wrote:
> The variables __gconv_path_elem, __gconv_max_path_elem_len and function
> __gconv_get_path declared in, as well as the type path_elem and macro
> GCONV_NCHAR_GOAL defined in gconv_int.h are all used in only one iconv
> compilation unit each. In addition, the extern declaration of the variable
> __gconv_nmodules refers to a variable that does not exist any more.
> Considering this, these symbols do not need to be exposed via a header file.
> 
> This patch removes the extern declarations from the header file and moves
> the definitions to the compilation units where they are used.

Ah! I had forgotten you had sent a distinct patch for this. I commented in
your other patch about this change.

> 2017-10-27  Arjun Shankar  <arjun@redhat.com>
> 
> 	* iconv/gconv_int.h (__gconv_path_elem): Remove.
> 	(__gconv_max_path_elem_len): Likewise.
> 	(__gconv_nmodules): Likewise.
> 	(__gconv_get_path): Likewise.
> 	(path_elem): Move to ...
> 	* iconv/gconv_conf.c: ... here.
> 	(__gconv_get_path): Mark function static.
> 	* iconv/gconv_int.h (GCONV_NCHAR_GOAL): Move to ...
> 	* iconv/gconv_open.c: ... here.

This looks good to me, please check this in when you're ready.

> 
> ---
>  iconv/gconv_conf.c |  9 ++++++++-
>  iconv/gconv_int.h  | 21 ---------------------
>  iconv/gconv_open.c |  4 ++++
>  3 files changed, 12 insertions(+), 22 deletions(-)
> 
> diff --git a/iconv/gconv_conf.c b/iconv/gconv_conf.c
> index d5cf86e..af23e34 100644
> --- a/iconv/gconv_conf.c
> +++ b/iconv/gconv_conf.c
> @@ -38,6 +38,13 @@
>  /* This is the default path where we look for module lists.  */
>  static const char default_gconv_path[] = GCONV_PATH;
>  
> +/* Type to represent search path.  */
> +struct path_elem
> +{
> +  const char *name;
> +  size_t len;
> +};
> +

OK.

>  /* The path elements, as determined by the __gconv_get_path function.
>     All path elements end in a slash.  */
>  struct path_elem *__gconv_path_elem;
> @@ -420,7 +427,7 @@ read_conf_file (const char *filename, const char *directory, size_t dir_len,
>  
>  
>  /* Determine the directories we are looking for data in.  */
> -void
> +static void
>  __gconv_get_path (void)

OK. Perfect.

>  {
>    /* Make sure we haven't done it already.  */
> diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h
> index e56757a..cc5022a 100644
> --- a/iconv/gconv_int.h
> +++ b/iconv/gconv_int.h
> @@ -28,19 +28,6 @@
>  __BEGIN_DECLS
>  
>  
> -/* Type to represent search path.  */
> -struct path_elem
> -{
> -  const char *name;
> -  size_t len;
> -};
> -
> -/* Variable with search path for `gconv' implementation.  */
> -extern struct path_elem *__gconv_path_elem attribute_hidden;
> -/* Maximum length of a single path element.  */
> -extern size_t __gconv_max_path_elem_len attribute_hidden;

OK.

> -
> -
>  /* Structure for alias definition.  Simply two strings.  */
>  struct gconv_alias
>  {
> @@ -49,10 +36,6 @@ struct gconv_alias
>  };
>  
>  
> -/* How many character should be converted in one call?  */
> -#define GCONV_NCHAR_GOAL	8160
> -

OK.

> -
>  /* Structure describing one loaded shared object.  This normally are
>     objects to perform conversation but as a special case the db shared
>     object is also handled.  */
> @@ -111,7 +94,6 @@ enum
>  extern void *__gconv_alias_db attribute_hidden;
>  
>  /* Array with available modules.  */
> -extern size_t __gconv_nmodules;

OK.

>  extern struct gconv_module *__gconv_modules_db attribute_hidden;
>  
>  /* Value of the GCONV_PATH environment variable.  */
> @@ -211,9 +193,6 @@ extern struct gconv_module *__gconv_get_modules_db (void);
>  /* Retrieve pointer to internal alias database.  */
>  extern void *__gconv_get_alias_db (void);
>  
> -/* Determine the directories we are looking in.  */
> -extern void __gconv_get_path (void) attribute_hidden;
> -

OK.

>  /* Comparison function to search alias.  */
>  extern int __gconv_alias_compare (const void *p1, const void *p2)
>       attribute_hidden;
> diff --git a/iconv/gconv_open.c b/iconv/gconv_open.c
> index d349527..f72f782 100644
> --- a/iconv/gconv_open.c
> +++ b/iconv/gconv_open.c
> @@ -26,6 +26,10 @@
>  #include <gconv_int.h>
>  
>  
> +/* How many character should be converted in one call?  */
> +#define GCONV_NCHAR_GOAL	8160

OK.

> +
> +
>  int
>  __gconv_open (const char *toset, const char *fromset, __gconv_t *handle,
>  	      int flags)
>

Patch
diff mbox series

diff --git a/iconv/gconv_conf.c b/iconv/gconv_conf.c
index d5cf86e..af23e34 100644
--- a/iconv/gconv_conf.c
+++ b/iconv/gconv_conf.c
@@ -38,6 +38,13 @@ 
 /* This is the default path where we look for module lists.  */
 static const char default_gconv_path[] = GCONV_PATH;
 
+/* Type to represent search path.  */
+struct path_elem
+{
+  const char *name;
+  size_t len;
+};
+
 /* The path elements, as determined by the __gconv_get_path function.
    All path elements end in a slash.  */
 struct path_elem *__gconv_path_elem;
@@ -420,7 +427,7 @@  read_conf_file (const char *filename, const char *directory, size_t dir_len,
 
 
 /* Determine the directories we are looking for data in.  */
-void
+static void
 __gconv_get_path (void)
 {
   /* Make sure we haven't done it already.  */
diff --git a/iconv/gconv_int.h b/iconv/gconv_int.h
index e56757a..cc5022a 100644
--- a/iconv/gconv_int.h
+++ b/iconv/gconv_int.h
@@ -28,19 +28,6 @@ 
 __BEGIN_DECLS
 
 
-/* Type to represent search path.  */
-struct path_elem
-{
-  const char *name;
-  size_t len;
-};
-
-/* Variable with search path for `gconv' implementation.  */
-extern struct path_elem *__gconv_path_elem attribute_hidden;
-/* Maximum length of a single path element.  */
-extern size_t __gconv_max_path_elem_len attribute_hidden;
-
-
 /* Structure for alias definition.  Simply two strings.  */
 struct gconv_alias
 {
@@ -49,10 +36,6 @@  struct gconv_alias
 };
 
 
-/* How many character should be converted in one call?  */
-#define GCONV_NCHAR_GOAL	8160
-
-
 /* Structure describing one loaded shared object.  This normally are
    objects to perform conversation but as a special case the db shared
    object is also handled.  */
@@ -111,7 +94,6 @@  enum
 extern void *__gconv_alias_db attribute_hidden;
 
 /* Array with available modules.  */
-extern size_t __gconv_nmodules;
 extern struct gconv_module *__gconv_modules_db attribute_hidden;
 
 /* Value of the GCONV_PATH environment variable.  */
@@ -211,9 +193,6 @@  extern struct gconv_module *__gconv_get_modules_db (void);
 /* Retrieve pointer to internal alias database.  */
 extern void *__gconv_get_alias_db (void);
 
-/* Determine the directories we are looking in.  */
-extern void __gconv_get_path (void) attribute_hidden;
-
 /* Comparison function to search alias.  */
 extern int __gconv_alias_compare (const void *p1, const void *p2)
      attribute_hidden;
diff --git a/iconv/gconv_open.c b/iconv/gconv_open.c
index d349527..f72f782 100644
--- a/iconv/gconv_open.c
+++ b/iconv/gconv_open.c
@@ -26,6 +26,10 @@ 
 #include <gconv_int.h>
 
 
+/* How many character should be converted in one call?  */
+#define GCONV_NCHAR_GOAL	8160
+
+
 int
 __gconv_open (const char *toset, const char *fromset, __gconv_t *handle,
 	      int flags)