[PATCH] ustr cleanups (policyrep branch)

[PATCH] ustr cleanups (policyrep branch)

[James Antill]

[-- Attachment #1.1: Type: text/plain, Size: 640 bytes --]


 Here are two cleanups for ustr usage within libsemanage on the
policyrep branch.

 The first is a corner case where you have two or more replacements in
the "replace_all" function of genhomedircon, previously one of those
multiple replacements (in theory) could fail due to malloc() returning
NULL and that would be missed. The fix probably makes the free test more
readable too.

 The second is that semanage_is_prefix() was previously defined by
calling ustr functions (inefficiently, even), and so had to allocate a
ustr to do it's work ... the fix just calls strncmp() directly.

-- 
James Antill <jantill@redhat.com>

[-- Attachment #1.2: selinux-semanage-policyrep-ustr-cleanups.patch --]
[-- Type: text/x-patch, Size: 1780 bytes --]

diff --git a/branches/policyrep/libsemanage/src/genhomedircon.c b/branches/policyrep/libsemanage/src/genhomedircon.c
index 68dedc8..f4cc16b 100644
--- a/branches/policyrep/libsemanage/src/genhomedircon.c
+++ b/branches/policyrep/libsemanage/src/genhomedircon.c
@@ -322,7 +322,7 @@ static semanage_list_t *make_template(genhomedircon_settings_t * s,
 static Ustr *replace_all(const char *str, const replacement_pair_t * repl)
 {
 	Ustr *retval = USTR_NULL;
-	int i, num_replaced = 0;
+	int i;
 
 	if (!str || !repl)
 		goto done;
@@ -330,10 +330,10 @@ static Ustr *replace_all(const char *str, const replacement_pair_t * repl)
 		goto done;
 
 	for (i = 0; repl[i].search_for; i++) {
-		num_replaced += ustr_replace_cstr(&retval, repl[i].search_for,
-						  repl[i].replace_with, 0);
+		ustr_replace_cstr(&retval, repl[i].search_for,
+				  repl[i].replace_with, 0);
 	}
-	if (!num_replaced)
+	if (ustr_enomem(retval))
 		ustr_sc_free(&retval);
 
       done:
diff --git a/branches/policyrep/libsemanage/src/utilities.c b/branches/policyrep/libsemanage/src/utilities.c
index 6a997e9..349a4be 100644
--- a/branches/policyrep/libsemanage/src/utilities.c
+++ b/branches/policyrep/libsemanage/src/utilities.c
@@ -60,22 +60,14 @@ char *semanage_findval(char *file, char *var, char *delim)
 
 int semanage_is_prefix(const char *str, const char *prefix)
 {
-	int retval;
-	Ustr *ustr = USTR_NULL;
-
 	if (!str) {
 		return FALSE;
 	}
 	if (!prefix) {
 		return TRUE;
 	}
-	if (!(ustr = ustr_dup_cstr(str))) {
-		return FALSE;
-	}
-	retval = (ustr_srch_cstr_fwd(ustr, 0, prefix) == 1);
-	ustr_sc_free(&ustr);
 
-	return retval;
+	return strncmp(str, prefix, strlen(prefix)) == 0;
 }
 
 char *semanage_split_on_space(const char *str)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

RE: [PATCH] ustr cleanups (policyrep branch)

[Todd Miller]

James Antill wrote:
>  Here are two cleanups for ustr usage within libsemanage on the
> policyrep branch.
> 
>  The first is a corner case where you have two or more replacements in
> the "replace_all" function of genhomedircon, previously one of those
> multiple replacements (in theory) could fail due to malloc() returning
> NULL and that would be missed. The fix probably makes the free test
> more readable too.
> 
>  The second is that semanage_is_prefix() was previously defined by
> calling ustr functions (inefficiently, even), and so had to allocate a
> ustr to do it's work ... the fix just calls strncmp() directly.

That looks sane to me.  Is there a reason you based this off of
policyrep instead of trunk?

 - todd


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: [PATCH] ustr cleanups (policyrep branch)

[James Antill]

[-- Attachment #1: Type: text/plain, Size: 443 bytes --]

On Mon, 2007-10-01 at 13:35 -0400, Todd Miller wrote:

> That looks sane to me.  Is there a reason you based this off of
> policyrep instead of trunk?

 Just me not having used SVN much, I did a search for ustr in the
branches directory and went from there ... didn't even notice the top
level trunk directory.
 The patch seems to apply without conflicts with -p3 from inside the
trunk dir.

-- 
James Antill <jantill@redhat.com>

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] ustr cleanups (policyrep branch)

[Stephen Smalley]

On Mon, 2007-10-01 at 02:46 -0400, James Antill wrote:
> Here are two cleanups for ustr usage within libsemanage on the
> policyrep branch.
> 
>  The first is a corner case where you have two or more replacements in
> the "replace_all" function of genhomedircon, previously one of those
> multiple replacements (in theory) could fail due to malloc() returning
> NULL and that would be missed. The fix probably makes the free test more
> readable too.
> 
>  The second is that semanage_is_prefix() was previously defined by
> calling ustr functions (inefficiently, even), and so had to allocate a
> ustr to do it's work ... the fix just calls strncmp() directly.

Thanks, merged on trunk.

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.