On 8/25/13 7:40 PM, Peter A. Bigot wrote:
From: "Peter A. Bigot" <[email protected]>
qemu.bbclass adds PSEUDO_UNLOAD=1 in qemu_run_binary to avoid reference to
pseudo functions that may not exist in the target environment. This patch
detects the addition of that variable within the environment to which the
call applies, even if not present in the parent environment.
As a side effect it fixes a memory leak.
Pseudo maintainer is on vacation this week.. so I'm not sure if he'll respond or
not. But I'll take a whack at it.
[YOCTO #4843]
Signed-off-by: Peter A. Bigot <[email protected]>
---
.../0001-pseudo_has_unload-add-function.patch | 190 ++++++++++++++++++++
meta/recipes-devtools/pseudo/pseudo_1.5.1.bb | 7 +-
2 files changed, 195 insertions(+), 2 deletions(-)
create mode 100644
meta/recipes-devtools/pseudo/files/0001-pseudo_has_unload-add-function.patch
diff --git
a/meta/recipes-devtools/pseudo/files/0001-pseudo_has_unload-add-function.patch
b/meta/recipes-devtools/pseudo/files/0001-pseudo_has_unload-add-function.patch
new file mode 100644
index 0000000..b5c81c9
--- /dev/null
+++
b/meta/recipes-devtools/pseudo/files/0001-pseudo_has_unload-add-function.patch
@@ -0,0 +1,190 @@
+From be97cb958f2934fa398fc8e344b25b84ebd4e90c Mon Sep 17 00:00:00 2001
+From: "Peter A. Bigot" <[email protected]>
+Date: Sun, 25 Aug 2013 19:22:09 -0500
+Subject: [PATCH] pseudo_has_unload: add function
+
+Various wrappers checked for a non-null pseudo_get_value("PSEUDO_UNLOAD") to
+determine whether the environment should include the pseudo variables. None
+of those checks freed the returned value when it was not null. The new
+check function does.
+
+The new check function also sees whether PSEUDO_UNLOAD was defined in the
+environment that should be used in the wrapped system call. This allows
+pkg_postinst scripts to strip out the LD_PRELOAD setting, for example before
+invoking qemu to execute commands in an environment that does not have
+libpseudo.so.
+
+[YOCTO #4843]
+
+Upstream-Status: Pending
+Signed-off-by: Peter A. Bigot <[email protected]>
+---
+ ports/common/guts/execv.c | 2 +-
+ ports/common/guts/execve.c | 2 +-
+ ports/common/guts/execvp.c | 2 +-
+ ports/common/guts/fork.c | 2 +-
+ ports/linux/newclone/pseudo_wrappers.c | 2 +-
+ ports/linux/oldclone/pseudo_wrappers.c | 2 +-
+ ports/unix/guts/popen.c | 2 +-
+ ports/unix/guts/system.c | 2 +-
+ pseudo.h | 1 +
+ pseudo_util.c | 27 +++++++++++++++++++++++++++
+ 10 files changed, 36 insertions(+), 8 deletions(-)
+
+diff --git a/ports/common/guts/execv.c b/ports/common/guts/execv.c
+index 763e1f9..3e1f820 100644
+--- a/ports/common/guts/execv.c
++++ b/ports/common/guts/execv.c
+@@ -19,7 +19,7 @@
+ }
+
+ pseudo_setupenv();
+- if (pseudo_get_value("PSEUDO_UNLOAD"))
++ if (pseudo_has_unload(NULL))
Ya, there is definitely a memory leak here.. the pseudo_get_value says:
/* Caller must free memory! */
char *
pseudo_get_value(const char *key) {
....
+diff --git a/pseudo_util.c b/pseudo_util.c
+index 8d0969e..16c70e0 100644
+--- a/pseudo_util.c
++++ b/pseudo_util.c
+@@ -95,6 +95,33 @@ dump_env(char **envp) {
+ }
+ #endif
+
++int
++pseudo_has_unload(char * const *envp) {
++ static const char unload[] = "PSEUDO_UNLOAD";
++ static size_t unload_len = strlen(unload);
++ size_t i = 0;
++
++ /* Is it in the caller environment? */
++ if (NULL != getenv(unload))
++ return 1;
We actually want to use the pseudo_get_value as that ensures that the internal
environment configuration is sane at all times. The issue is that applications
can do odd things to the environment, and just checking the environment itself
may not always give sane results.
The rule for variables is:
* PSEUDO Variables are loaded ASAP into a cache. This way if the user or
program clear or modify the environment we maintain the variable.
* To 'reset' the variables to different values an exec needs to be performed.
(Prior to the exec we also restore any missing variables that have been cached
to the environment.)
-however- PSEUDO_UNLOAD is supposed to be special and acted on at the vary next
exec... So something like the following:
int
pseudo_has_unload(char * const *envp) {
static const char unload[] = "PSEUDO_UNLOAD";
static size_t unload_len = strlen(unload);
char * value = NULL;
size_t i = 0;
value = pseudo_get_value(unload)
if (value) {
free(value);
return 1;
}
free(value);
/* Is it in the caller environment? */
if (NULL != getenv(unload))
return 1;
/* Is it in the operational environment? */
while (envp && *envp) {
if ((!strncmp(*envp, unload, unload_len)) && ('=' ==
(*envp)[unload_len]))
return 1;
++envp;
}
return 0;
}
This is -almost- the same as what you had. The difference is that we check the
cached value first, and use the get_value function which does all of the setup
and comparison using the common function. This ensures that the environment
cache is properly configured as early as possible, only one accessor of the
cache is used and it also checks the running and operation cache for the
alternative value (in the same way your patch did it).
--Mark
_______________________________________________
Openembedded-core mailing list
[email protected]
http://lists.openembedded.org/mailman/listinfo/openembedded-core
