[PATCH] Make global_iconv_handle static to the C source file defining it.
Jeremy Allison
jra at samba.org
Mon Apr 17 21:52:56 UTC 2017
Another cleanup. By judicious use of utility
functions this allows global_iconv_handle to
become a static pointer only accessed inside
lib/util/charset/codepoints.c.
As a bonus it also allows removal of another
use of talloc_autofree_context().
Please review and push if happy !
Cheers,
Jeremy.
-------------- next part --------------
From 64b0f33330b6ff466e4099bce22b0a7dfc23535f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:31:17 -0700
Subject: [PATCH 1/8] lib: Remove smb_iconv_handle_reinit_lp().
It's merely a wrapper for smb_iconv_handle_reinit(),
only used in one place and smb_iconv_handle_reinit()
is already called from lib/param/loadparm.c.
Removing this will make it easier to make global_iconv_handle
private state to lib/util/charset/codepoints.c later.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/param/loadparm.c | 6 +++++-
lib/param/param.h | 4 ----
lib/param/util.c | 11 -----------
3 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index 4d21d88cc6c..b6bac073d36 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -3357,7 +3357,11 @@ _PUBLIC_ void reload_charcnv(struct loadparm_context *lp_ctx)
if (old_ic == NULL) {
old_ic = global_iconv_handle;
}
- lp_ctx->iconv_handle = smb_iconv_handle_reinit_lp(lp_ctx, lp_ctx, old_ic);
+ lp_ctx->iconv_handle = smb_iconv_handle_reinit(lp_ctx,
+ lpcfg_dos_charset(lp_ctx),
+ lpcfg_unix_charset(lp_ctx),
+ true,
+ old_ic);
global_iconv_handle = lp_ctx->iconv_handle;
}
diff --git a/lib/param/param.h b/lib/param/param.h
index e123e67a990..a6dbafa42f9 100644
--- a/lib/param/param.h
+++ b/lib/param/param.h
@@ -301,10 +301,6 @@ char *smbd_tmp_path(TALLOC_CTX *mem_ctx,
const char *lpcfg_imessaging_path(TALLOC_CTX *mem_ctx,
struct loadparm_context *lp_ctx);
-struct smb_iconv_handle *smb_iconv_handle_reinit_lp(TALLOC_CTX *mem_ctx,
- struct loadparm_context *lp_ctx,
- struct smb_iconv_handle *old_ic);
-
const char *lpcfg_sam_name(struct loadparm_context *lp_ctx);
const char *lpcfg_sam_dnsname(struct loadparm_context *lp_ctx);
diff --git a/lib/param/util.c b/lib/param/util.c
index 233981abfa8..52796562ec5 100644
--- a/lib/param/util.c
+++ b/lib/param/util.c
@@ -248,17 +248,6 @@ const char *lpcfg_imessaging_path(TALLOC_CTX *mem_ctx,
return smbd_tmp_path(mem_ctx, lp_ctx, "msg");
}
-struct smb_iconv_handle *smb_iconv_handle_reinit_lp(TALLOC_CTX *mem_ctx,
- struct loadparm_context *lp_ctx,
- struct smb_iconv_handle *old_ic)
-{
- return smb_iconv_handle_reinit(mem_ctx, lpcfg_dos_charset(lp_ctx),
- lpcfg_unix_charset(lp_ctx),
- true,
- old_ic);
-}
-
-
const char *lpcfg_sam_name(struct loadparm_context *lp_ctx)
{
switch (lpcfg_server_role(lp_ctx)) {
--
2.12.2.762.g0e3151a226-goog
From ea9d01b6eaaed005b6d85c5f1e218f22cc5c6237 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:42:39 -0700
Subject: [PATCH 2/8] lib: charset: Add utility functions reinit_iconv_handle()
and free_iconv_handle(void).
Not yet used. Will enable us to make global_iconv_handle private.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/util/charset/charset.h | 5 +++++
lib/util/charset/codepoints.c | 17 +++++++++++++++++
2 files changed, 22 insertions(+)
diff --git a/lib/util/charset/charset.h b/lib/util/charset/charset.h
index ca7a4377455..750071491f0 100644
--- a/lib/util/charset/charset.h
+++ b/lib/util/charset/charset.h
@@ -170,6 +170,11 @@ struct smb_iconv_handle *get_iconv_testing_handle(TALLOC_CTX *mem_ctx,
const char *dos_charset,
const char *unix_charset,
bool use_builtin_handlers);
+struct smb_iconv_handle *reinit_iconv_handle(TALLOC_CTX *mem_ctx,
+ const char *dos_charset,
+ const char *unix_charset);
+void free_iconv_handle(void);
+
smb_iconv_t get_conv_handle(struct smb_iconv_handle *ic,
charset_t from, charset_t to);
const char *charset_name(struct smb_iconv_handle *ic, charset_t ch);
diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
index cfb0b27fcc1..55c47585f8f 100644
--- a/lib/util/charset/codepoints.c
+++ b/lib/util/charset/codepoints.c
@@ -16521,6 +16521,23 @@ struct smb_iconv_handle *get_iconv_testing_handle(TALLOC_CTX *mem_ctx,
dos_charset, unix_charset, use_builtin_handlers, NULL);
}
+struct smb_iconv_handle *reinit_iconv_handle(TALLOC_CTX *mem_ctx,
+ const char *dos_charset,
+ const char *unix_charset)
+{
+ global_iconv_handle = smb_iconv_handle_reinit(mem_ctx,
+ dos_charset,
+ unix_charset,
+ true,
+ global_iconv_handle);
+ return global_iconv_handle;
+}
+
+void free_iconv_handle(void)
+{
+ TALLOC_FREE(global_iconv_handle);
+}
+
/**
* Return the name of a charset to give to iconv().
**/
--
2.12.2.762.g0e3151a226-goog
From 6bee9f4e4ed33b6beedbc1e46eb72f79a40876ae Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:44:08 -0700
Subject: [PATCH 3/8] s3: lib: charcnv. Remove use of global
global_iconv_handle.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/lib/charcnv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/source3/lib/charcnv.c b/source3/lib/charcnv.c
index 66a98f95ab6..5ec070c0b6c 100644
--- a/source3/lib/charcnv.c
+++ b/source3/lib/charcnv.c
@@ -27,7 +27,7 @@
**/
void gfree_charcnv(void)
{
- TALLOC_FREE(global_iconv_handle);
+ free_iconv_handle();
}
/**
--
2.12.2.762.g0e3151a226-goog
From 23bf630be82957a034be859981c469996371c0be Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:47:17 -0700
Subject: [PATCH 4/8] s3: param: Use new utility function to hide use of
global_iconv_handle.
Add error return check.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
source3/param/loadparm.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index b543a6f48c9..d790965e68e 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -2368,9 +2368,12 @@ bool lp_file_list_changed(void)
**/
static void init_iconv(void)
{
- global_iconv_handle = smb_iconv_handle_reinit(NULL, lp_dos_charset(),
- lp_unix_charset(),
- true, global_iconv_handle);
+ struct smb_iconv_handle *ret = reinit_iconv_handle(NULL,
+ lp_dos_charset(),
+ lp_unix_charset());
+ if (ret == NULL) {
+ smb_panic("reinit_iconv_handle failed");
+ }
}
/***************************************************************************
--
2.12.2.762.g0e3151a226-goog
From 7c454d87e900b0e4433d183d7636fc384418843a Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:51:17 -0700
Subject: [PATCH 5/8] lib: param: Use utility functions to get rid of two more
uses of global_iconv_handle.
Add error return checking.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/param/loadparm.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index b6bac073d36..b7d830306d0 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -1265,10 +1265,13 @@ bool handle_charset(struct loadparm_context *lp_ctx, struct loadparm_service *se
{
if (lp_ctx->s3_fns) {
if (*ptr == NULL || strcmp(*ptr, pszParmValue) != 0) {
- global_iconv_handle = smb_iconv_handle_reinit(NULL,
- lpcfg_dos_charset(lp_ctx),
- lpcfg_unix_charset(lp_ctx),
- true, global_iconv_handle);
+ struct smb_iconv_handle *ret =
+ reinit_iconv_handle(NULL,
+ lpcfg_dos_charset(lp_ctx),
+ lpcfg_unix_charset(lp_ctx));
+ if (ret == NULL) {
+ smb_panic("reinit_iconv_handle failed");
+ }
}
}
@@ -1303,16 +1306,19 @@ bool handle_dos_charset(struct loadparm_context *lp_ctx, struct loadparm_service
}
if (*ptr == NULL || strcmp(*ptr, pszParmValue) != 0) {
+ struct smb_iconv_handle *ret = NULL;
if (is_utf8) {
DEBUG(0,("ERROR: invalid DOS charset: 'dos charset' must not "
"be UTF8, using (default value) %s instead.\n",
DEFAULT_DOS_CHARSET));
pszParmValue = DEFAULT_DOS_CHARSET;
}
- global_iconv_handle = smb_iconv_handle_reinit(NULL,
- lpcfg_dos_charset(lp_ctx),
- lpcfg_unix_charset(lp_ctx),
- true, global_iconv_handle);
+ ret = reinit_iconv_handle(NULL,
+ lpcfg_dos_charset(lp_ctx),
+ lpcfg_unix_charset(lp_ctx));
+ if (ret == NULL) {
+ smb_panic("reinit_iconv_handle failed");
+ }
}
}
--
2.12.2.762.g0e3151a226-goog
From 8ef9dac5268c1d4c3a94bd3d23404926c22c14e8 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 15:57:28 -0700
Subject: [PATCH 6/8] lib: param: Remove the last external use of
global_iconv_handle by calling the utility function reinit_iconv_handle().
Add an error check.
This *looks* like a logic change, but it is not.
The only change is the addition of the error return check.
The reason is that the changed function, reload_charcnv(),
is the *only* function that sets lp_ctx->iconv_handle. And
it does so just before setting global_iconv_handle = lp_ctx->iconv_handle.
Calling the utility function reinit_iconv_handle()
instead merely sets global_iconv_handle first, then
assigns it (as the return) to lp_ctx->iconv_handle.
So all this is doing is reversing the order of
setting global_iconv_handle and lp_ctx->iconv_handle
to the same thing.
Even the removal of the lines:
- struct smb_iconv_handle *old_ic = lp_ctx->iconv_handle
- if (old_ic == NULL) {
- old_ic = global_iconv_handle;
has no effect, as remember that lp_ctx->iconv_handle
is only ever set to the same value as global_iconv_handle,
and once this function has been run once, lp_ctx->iconv_handle != NULL.
This allows us finally to make global_iconv_handle private
to the C source file that defines it.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/param/loadparm.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/lib/param/loadparm.c b/lib/param/loadparm.c
index b7d830306d0..0a4549b6d85 100644
--- a/lib/param/loadparm.c
+++ b/lib/param/loadparm.c
@@ -3355,20 +3355,16 @@ struct smb_iconv_handle *lpcfg_iconv_handle(struct loadparm_context *lp_ctx)
_PUBLIC_ void reload_charcnv(struct loadparm_context *lp_ctx)
{
- struct smb_iconv_handle *old_ic = lp_ctx->iconv_handle;
if (!lp_ctx->global) {
return;
}
- if (old_ic == NULL) {
- old_ic = global_iconv_handle;
+ lp_ctx->iconv_handle = reinit_iconv_handle(lp_ctx,
+ lpcfg_dos_charset(lp_ctx),
+ lpcfg_unix_charset(lp_ctx));
+ if (lp_ctx->iconv_handle == NULL) {
+ smb_panic("reinit_iconv_handle failed");
}
- lp_ctx->iconv_handle = smb_iconv_handle_reinit(lp_ctx,
- lpcfg_dos_charset(lp_ctx),
- lpcfg_unix_charset(lp_ctx),
- true,
- old_ic);
- global_iconv_handle = lp_ctx->iconv_handle;
}
_PUBLIC_ char *lpcfg_tls_keyfile(TALLOC_CTX *mem_ctx, struct loadparm_context *lp_ctx)
--
2.12.2.762.g0e3151a226-goog
From 1e7ddbaacaf1160d4563abce269f7587f6feae83 Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 16:05:02 -0700
Subject: [PATCH 7/8] lib: charset: Make global_iconv_handle private.
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/util/charset/charset.h | 1 -
lib/util/charset/codepoints.c | 2 +-
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/lib/util/charset/charset.h b/lib/util/charset/charset.h
index 750071491f0..ff466c34bb9 100644
--- a/lib/util/charset/charset.h
+++ b/lib/util/charset/charset.h
@@ -164,7 +164,6 @@ bool convert_string_error(charset_t from, charset_t to,
void *dest, size_t destlen,
size_t *converted_size);
-extern struct smb_iconv_handle *global_iconv_handle;
struct smb_iconv_handle *get_iconv_handle(void);
struct smb_iconv_handle *get_iconv_testing_handle(TALLOC_CTX *mem_ctx,
const char *dos_charset,
diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
index 55c47585f8f..956e4b2ad7b 100644
--- a/lib/util/charset/codepoints.c
+++ b/lib/util/charset/codepoints.c
@@ -16502,7 +16502,7 @@ struct smb_iconv_handle {
smb_iconv_t conv_handles[NUM_CHARSETS][NUM_CHARSETS];
};
-struct smb_iconv_handle *global_iconv_handle = NULL;
+static struct smb_iconv_handle *global_iconv_handle = NULL;
struct smb_iconv_handle *get_iconv_handle(void)
{
--
2.12.2.762.g0e3151a226-goog
From 714b0f484fc0bab43e6522a32a71bf55e724522f Mon Sep 17 00:00:00 2001
From: Jeremy Allison <jra at samba.org>
Date: Tue, 11 Apr 2017 16:06:08 -0700
Subject: [PATCH 8/8] lib: charset: Remove use of talloc_autofree_context() for
global_iconv_handle.
All other callers use NULL here anyway, so there's no
need to use a special context for get_iconv_handle().
Signed-off-by: Jeremy Allison <jra at samba.org>
---
lib/util/charset/codepoints.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
index 956e4b2ad7b..213743e8e77 100644
--- a/lib/util/charset/codepoints.c
+++ b/lib/util/charset/codepoints.c
@@ -16507,7 +16507,7 @@ static struct smb_iconv_handle *global_iconv_handle = NULL;
struct smb_iconv_handle *get_iconv_handle(void)
{
if (global_iconv_handle == NULL)
- global_iconv_handle = smb_iconv_handle_reinit(talloc_autofree_context(),
+ global_iconv_handle = smb_iconv_handle_reinit(NULL,
"ASCII", "UTF-8", true, NULL);
return global_iconv_handle;
}
--
2.12.2.762.g0e3151a226-goog
More information about the samba-technical
mailing list