[PATCH 0/7] Remove ustr library dependency

[PATCH 0/7] Remove ustr library dependency

[Nicolas Iooss]

Hello,

ustr library uses old (pre-C99) "extern inline" semantic. This makes it
incompatible with recent versions of gcc and clang, which default to
C99 standard. Distributions have shipped patched versions of this
library to fix issues [1] but there is no upstream solution to make ustr
compatible with C99 standard. Moreover the git tree of ustr
(http://www.and.org/ustr/ustr.git) has not been updated since 2008 and
the developer of this project did not reply to emails.

This patchset aims at removing ustr from SELinux userland libraries and
tools. The library is currenlty only used by genhomedircon functions in
libsemanage. Here is how the patches are organised:

* patches 1 and 2 make libsemanage test infrastructure work properly,
* patch 3 expands some test cases in order to better check the
  modifications of the next patch,
* patch 4 reimplements functions in libsemanage/src/utilities.c without
  ustr,
* patches 5 and 6 remove ustr from libsemanage/src/genhomedircon.c (I
  introduced a new function, semanage_str_replace(), to replace
  ustr_replace_cstr()),
* patch 7 removes references to ustr in Makefile, README and pkg-config
  files.

Thanks,
Nicolas

[1] for example Gentoo package uses this patch:
https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/files/ustr-1.0.4-gcc_5-check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0

Nicolas Iooss (7):
  libsemanage/tests: make "make test" fail when a CUnit test fails
  libsemanage/tests: make tests standalone
  libsemanage/tests: test more cases of semanage_split*()
  libsemanage: simplify string utilities functions
  libsemanage: add semanage_str_replace() utility function
  libsemanage: genhomedircon: drop ustr dependency
  libsemanage: remove ustr library from Makefiles and pkg-config

 README                                |   2 +-
 libsemanage/src/Makefile              |   2 +-
 libsemanage/src/genhomedircon.c       | 137 ++++++++++++++++------------------
 libsemanage/src/libsemanage.pc.in     |   2 +-
 libsemanage/src/utilities.c           | 119 +++++++++++++++++------------
 libsemanage/src/utilities.h           |  10 +++
 libsemanage/tests/Makefile            |  14 ++--
 libsemanage/tests/libsemanage-tests.c |   9 ++-
 libsemanage/tests/test_utilities.c    |  85 ++++++++++++++++-----
 9 files changed, 232 insertions(+), 148 deletions(-)

-- 
2.11.0

[PATCH 1/7] libsemanage/tests: make "make test" fail when a CUnit test fails

[Nicolas Iooss]

When modifications to libsemanage functions break the test cases tested
with the CUnit framework, "make test" currently succeeds, even though it
prints an output similar to:

    Suite: semanage_store
      Test: semanage_store_access_check ...passed
      Test: semanage_get_lock ...passed
      Test: semanage_nc_sort ...passed
    Suite: semanage_utilities
      Test: semanage_is_prefix ...passed
      Test: semanage_split_on_space ...FAILED
        1. test_utilities.c:150  - CU_ASSERT_STRING_EQUAL(temp,"baz")
      Test: semanage_split ...passed
      Test: semanage_list ...passed
      Test: semanage_str_count ...passed
      Test: semanage_rtrim ...passed
      Test: semanage_str_replace ...passed
      Test: semanage_findval ...passed
      Test: slurp_file_filter ...passed

Like commit 2489b50a9162 ("libsepol: make "make test" fails when a CUnit
test fails") did for libsepol tests, modify the logic of function
do_tests() to return an error value when there has been at least one
failure. This makes "make test" fail as expected.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/tests/libsemanage-tests.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/libsemanage/tests/libsemanage-tests.c b/libsemanage/tests/libsemanage-tests.c
index 735d36fd9074..048751b8b172 100644
--- a/libsemanage/tests/libsemanage-tests.c
+++ b/libsemanage/tests/libsemanage-tests.c
@@ -26,6 +26,7 @@
 #include <CUnit/Console.h>
 #include <CUnit/TestDB.h>
 
+#include <stdbool.h>
 #include <stdio.h>
 #include <getopt.h>
 #include <stdlib.h>
@@ -47,9 +48,10 @@ static void usage(char *progname)
 	printf("\t-i, --interactive\t\tinteractive console\n");
 }
 
-static int do_tests(int interactive, int verbose)
+static bool do_tests(int interactive, int verbose)
 {
 	CU_pSuite suite = NULL;
+	unsigned int num_failures;
 
 	/* Initialize the CUnit test registry. */
 	if (CUE_SUCCESS != CU_initialize_registry())
@@ -67,8 +69,9 @@ static int do_tests(int interactive, int verbose)
 		CU_console_run_tests();
 	else
 		CU_basic_run_tests();
+	num_failures = CU_get_number_of_tests_failed();
 	CU_cleanup_registry();
-	return CU_get_error();
+	return CU_get_error() == CUE_SUCCESS && num_failures == 0;
 
 }
 
@@ -101,7 +104,7 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (do_tests(interactive, verbose))
+	if (!do_tests(interactive, verbose))
 		return -1;
 
 	return 0;
-- 
2.11.0

[PATCH 2/7] libsemanage/tests: make tests standalone

[Nicolas Iooss]

In order to run libsemanage tests, libsepol and libselinux source
directories need to exist next to libsemanage source directory. This
prevents tests to be run when using the released package.

As libsemanage tests only use public API of libselinux and libsepol,
link with the shared objects which are likely to be installed on the
system (or at least present in $DESTDIR).

While at it, drop TESTSRC variable as it was used to find libsemanage
internal headers but not the tested library (libsemanage.a). Moreover
add ../src/libsemanage.a to the target dependencies of the test
executable in order to rebuild it after libsemanage.a has been updated.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/tests/Makefile | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/libsemanage/tests/Makefile b/libsemanage/tests/Makefile
index 4b81fed70660..bd6a5fd74906 100644
--- a/libsemanage/tests/Makefile
+++ b/libsemanage/tests/Makefile
@@ -1,23 +1,23 @@
+PREFIX ?= $(DESTDIR)/usr
+LIBDIR ?= $(PREFIX)/lib
+
 # Add your test source files here:
 SOURCES = $(wildcard *.c)
 
-# Point this variable to the libsemanage source directory you want to test:
-TESTSRC=../src
-
 # Add the required external object files here:
-LIBS = ../src/libsemanage.a ../../libselinux/src/libselinux.a ../../libsepol/src/libsepol.a
+LIBS = ../src/libsemanage.a -L$(LIBDIR) -lselinux -lsepol
 
 ###########################################################################
 
 EXECUTABLE = libsemanage-tests
 CFLAGS += -g -O0 -Wall -W -Wundef -Wmissing-noreturn -Wmissing-format-attribute -Wno-unused-parameter
-INCLUDE = -I$(TESTSRC) -I$(TESTSRC)/../include
+INCLUDE = -I../src -I../include
 LDFLAGS += -lcunit -lustr -lbz2 -laudit
 OBJECTS = $(SOURCES:.c=.o) 
 
 all: $(EXECUTABLE) 
 
-$(EXECUTABLE): $(OBJECTS) 
+$(EXECUTABLE): $(OBJECTS) ../src/libsemanage.a
 	$(CC) $(OBJECTS) $(LIBS) $(LDFLAGS) -o $@
 
 %.o: %.c
-- 
2.11.0

[PATCH 3/7] libsemanage/tests: test more cases of semanage_split*()

[Nicolas Iooss]

Before modifying semanage_split_on_space() and semanage_split(), test in
test_utilities.c how these functions behave for example when several
delimiter tokens are concatenated in the input string.

While at it, fix the memory leaks which were present in libsemanage
tests.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/tests/test_utilities.c | 51 ++++++++++++++++++++++++--------------
 1 file changed, 33 insertions(+), 18 deletions(-)

diff --git a/libsemanage/tests/test_utilities.c b/libsemanage/tests/test_utilities.c
index 32cc33c1b510..27fa70dc2a57 100644
--- a/libsemanage/tests/test_utilities.c
+++ b/libsemanage/tests/test_utilities.c
@@ -129,7 +129,7 @@ void test_semanage_is_prefix(void)
 
 void test_semanage_split_on_space(void)
 {
-	char *str = strdup("foo bar baz");
+	char *str = strdup("   foo   bar    baz");
 	char *temp;
 
 	if (!str) {
@@ -137,24 +137,24 @@ void test_semanage_split_on_space(void)
 		    ("semanage_split_on_space: unable to perform test, no memory");
 	}
 	temp = semanage_split_on_space(str);
-	if (strncmp(temp, "bar", 3)) {
-		CU_FAIL("semanage_split_on_space: token did not match");
-	}
-	temp = semanage_split_on_space(temp);
-	if (strncmp(temp, "baz", 3)) {
-		CU_FAIL("semanage_split_on_space: token did not match");
-	}
-	temp = semanage_split_on_space(temp);
-	if (strcmp(temp, "")) {
-		CU_FAIL("semanage_split_on_space: token did not match");
-	}
+	CU_ASSERT_STRING_EQUAL(temp, "bar    baz");
+	free(str);
+	str = temp;
+
+	temp = semanage_split_on_space(str);
+	CU_ASSERT_STRING_EQUAL(temp, "baz");
+	free(str);
+	str = temp;
 
+	temp = semanage_split_on_space(str);
+	CU_ASSERT_STRING_EQUAL(temp, "");
 	free(str);
+	free(temp);
 }
 
 void test_semanage_split(void)
 {
-	char *str = strdup("foo1 foo2 foo:bar");
+	char *str = strdup("foo1 foo2   foo::bar:::");
 	char *temp;
 
 	if (!str) {
@@ -163,13 +163,24 @@ void test_semanage_split(void)
 		return;
 	}
 	temp = semanage_split(str, NULL);
-	CU_ASSERT_NSTRING_EQUAL(temp, "foo2", 4);
-	temp = semanage_split(temp, "");
-	CU_ASSERT_NSTRING_EQUAL(temp, "foo", 3);
-	temp = semanage_split(temp, ":");
-	CU_ASSERT_NSTRING_EQUAL(temp, "bar", 3);
+	CU_ASSERT_STRING_EQUAL(temp, "foo2   foo::bar:::");
+	free(str);
+	str = temp;
 
+	temp = semanage_split(str, "");
+	CU_ASSERT_STRING_EQUAL(temp, "foo::bar:::");
 	free(str);
+	str = temp;
+
+	temp = semanage_split(str, ":");
+	CU_ASSERT_STRING_EQUAL(temp, "bar:::");
+	free(str);
+	str = temp;
+
+	temp = semanage_split(str, ":");
+	CU_ASSERT_STRING_EQUAL(temp, "");
+	free(str);
+	free(temp);
 }
 
 void test_semanage_list(void)
@@ -242,6 +253,8 @@ void test_semanage_rtrim(void)
 	CU_ASSERT_STRING_EQUAL(str, "/blah/foo/bar/b");
 	semanage_rtrim(str, '/');
 	CU_ASSERT_STRING_EQUAL(str, "/blah/foo/bar");
+
+	free(str);
 }
 
 void test_semanage_findval(void)
@@ -252,6 +265,7 @@ void test_semanage_findval(void)
 	}
 	tok = semanage_findval(fname, "one", NULL);
 	CU_ASSERT_STRING_EQUAL(tok, "");
+	free(tok);
 	rewind(fptr);
 	tok = semanage_findval(fname, "one", "");
 	CU_ASSERT_STRING_EQUAL(tok, "");
@@ -259,6 +273,7 @@ void test_semanage_findval(void)
 	rewind(fptr);
 	tok = semanage_findval(fname, "sigma", "=");
 	CU_ASSERT_STRING_EQUAL(tok, "foo");
+	free(tok);
 }
 
 int PREDICATE(const char *str)
-- 
2.11.0

[PATCH 4/7] libsemanage: simplify string utilities functions

[Nicolas Iooss]

Use string functions from C standard library instead of ustr. This makes
the code simpler and make utilities.c no longer depend on ustr library.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/src/utilities.c | 64 +++++++++++++--------------------------------
 1 file changed, 18 insertions(+), 46 deletions(-)

diff --git a/libsemanage/src/utilities.c b/libsemanage/src/utilities.c
index f48ffa489d14..b5b92b085310 100644
--- a/libsemanage/src/utilities.c
+++ b/libsemanage/src/utilities.c
@@ -26,7 +26,6 @@
 #include <string.h>
 #include <sys/types.h>
 #include <assert.h>
-#include <ustr.h>
 
 #define TRUE 1
 #define FALSE 0
@@ -74,64 +73,37 @@ char *semanage_split_on_space(const char *str)
 {
 	/* as per the man page, these are the isspace() chars */
 	const char *seps = "\f\n\r\t\v ";
-	size_t slen = strlen(seps);
-	size_t off = 0, rside_len = 0;
-	char *retval = NULL;
-	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
+	size_t off = 0;
 
 	if (!str)
-		goto done;
-	if (!(ustr = ustr_dup_cstr(str)))
-		goto done;
-	temp =
-	    ustr_split_spn_chrs(ustr, &off, seps, slen, USTR_NULL,
-				USTR_FLAG_SPLIT_DEF);
-	if (!temp)
-		goto done;
-	/* throw away the left hand side */
-	ustr_sc_free(&temp);
-
-	rside_len = ustr_len(ustr) - off;
-	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
-	if (!temp)
-		goto done;
-	retval = strdup(ustr_cstr(temp));
-	ustr_sc_free(&temp);
+		return NULL;
 
-      done:
-	ustr_sc_free(&ustr);
-	return retval;
+	/* skip one token and the spaces before and after it */
+	off = strspn(str, seps);
+	off += strcspn(str + off, seps);
+	off += strspn(str + off, seps);
+	return strdup(str + off);
 }
 
 char *semanage_split(const char *str, const char *delim)
 {
-	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
-	size_t off = 0, rside_len = 0;
-	char *retval = NULL;
+	char *retval;
+	size_t delim_len;
 
 	if (!str)
-		goto done;
+		return NULL;
 	if (!delim || !(*delim))
 		return semanage_split_on_space(str);
-	ustr = ustr_dup_cstr(str);
-	temp =
-	    ustr_split_cstr(ustr, &off, delim, USTR_NULL, USTR_FLAG_SPLIT_DEF);
-	if (!temp)
-		goto done;
-	/* throw away the left hand side */
-	ustr_sc_free(&temp);
-
-	rside_len = ustr_len(ustr) - off;
 
-	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
-	if (!temp)
-		goto done;
-	retval = strdup(ustr_cstr(temp));
-	ustr_sc_free(&temp);
+	retval = strstr(str, delim);
+	if (retval == NULL)
+		return NULL;
 
-      done:
-	ustr_sc_free(&ustr);
-	return retval;
+	delim_len = strlen(delim);
+	do {
+		retval += delim_len;
+	} while (!strncmp(retval, delim, delim_len));
+	return strdup(retval);
 }
 
 int semanage_list_push(semanage_list_t ** list, const char *data)
-- 
2.11.0

Re: [PATCH 4/7] libsemanage: simplify string utilities functions

[Stephen Smalley]

On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
> Use string functions from C standard library instead of ustr. This
> makes
> the code simpler and make utilities.c no longer depend on ustr
> library.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
>  libsemanage/src/utilities.c | 64 +++++++++++++--------------------
> ------------
>  1 file changed, 18 insertions(+), 46 deletions(-)
> 
> diff --git a/libsemanage/src/utilities.c
> b/libsemanage/src/utilities.c
> index f48ffa489d14..b5b92b085310 100644
> --- a/libsemanage/src/utilities.c
> +++ b/libsemanage/src/utilities.c
> @@ -26,7 +26,6 @@
>  #include <string.h>
>  #include <sys/types.h>
>  #include <assert.h>
> -#include <ustr.h>
>  
>  #define TRUE 1
>  #define FALSE 0
> @@ -74,64 +73,37 @@ char *semanage_split_on_space(const char *str)
>  {
>  	/* as per the man page, these are the isspace() chars */
>  	const char *seps = "\f\n\r\t\v ";
> -	size_t slen = strlen(seps);
> -	size_t off = 0, rside_len = 0;
> -	char *retval = NULL;
> -	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
> +	size_t off = 0;
>  
>  	if (!str)
> -		goto done;
> -	if (!(ustr = ustr_dup_cstr(str)))
> -		goto done;
> -	temp =
> -	    ustr_split_spn_chrs(ustr, &off, seps, slen, USTR_NULL,
> -				USTR_FLAG_SPLIT_DEF);
> -	if (!temp)
> -		goto done;
> -	/* throw away the left hand side */
> -	ustr_sc_free(&temp);
> -
> -	rside_len = ustr_len(ustr) - off;
> -	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
> -	if (!temp)
> -		goto done;
> -	retval = strdup(ustr_cstr(temp));
> -	ustr_sc_free(&temp);
> +		return NULL;
>  
> -      done:
> -	ustr_sc_free(&ustr);
> -	return retval;
> +	/* skip one token and the spaces before and after it */
> +	off = strspn(str, seps);
> +	off += strcspn(str + off, seps);
> +	off += strspn(str + off, seps);
> +	return strdup(str + off);
>  }
>  
>  char *semanage_split(const char *str, const char *delim)
>  {
> -	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
> -	size_t off = 0, rside_len = 0;
> -	char *retval = NULL;
> +	char *retval;
> +	size_t delim_len;
>  
>  	if (!str)
> -		goto done;
> +		return NULL;
>  	if (!delim || !(*delim))
>  		return semanage_split_on_space(str);
> -	ustr = ustr_dup_cstr(str);
> -	temp =
> -	    ustr_split_cstr(ustr, &off, delim, USTR_NULL,
> USTR_FLAG_SPLIT_DEF);
> -	if (!temp)
> -		goto done;
> -	/* throw away the left hand side */
> -	ustr_sc_free(&temp);
> -
> -	rside_len = ustr_len(ustr) - off;
>  
> -	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
> -	if (!temp)
> -		goto done;
> -	retval = strdup(ustr_cstr(temp));
> -	ustr_sc_free(&temp);
> +	retval = strstr(str, delim);
> +	if (retval == NULL)
> +		return NULL;
>  
> -      done:
> -	ustr_sc_free(&ustr);
> -	return retval;
> +	delim_len = strlen(delim);
> +	do {
> +		retval += delim_len;
> +	} while (!strncmp(retval, delim, delim_len));

This seems unnecessarily complicated.  Do we expect this to ever be
called on a string containing repeating instances of the delimeter?

> +	return strdup(retval);
>  }
>  
>  int semanage_list_push(semanage_list_t ** list, const char *data)

Re: [PATCH 4/7] libsemanage: simplify string utilities functions

[Nicolas Iooss]

On 19/12/16 16:46, Stephen Smalley wrote:
> On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
>> Use string functions from C standard library instead of ustr. This
>> makes
>> the code simpler and make utilities.c no longer depend on ustr
>> library.
>>
>> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>> ---
>>  libsemanage/src/utilities.c | 64 +++++++++++++--------------------
>> ------------
>>  1 file changed, 18 insertions(+), 46 deletions(-)
>>
>> diff --git a/libsemanage/src/utilities.c
>> b/libsemanage/src/utilities.c
>> index f48ffa489d14..b5b92b085310 100644
>> --- a/libsemanage/src/utilities.c
>> +++ b/libsemanage/src/utilities.c
>> @@ -26,7 +26,6 @@
>>  #include <string.h>
>>  #include <sys/types.h>
>>  #include <assert.h>
>> -#include <ustr.h>
>>  
>>  #define TRUE 1
>>  #define FALSE 0
>> @@ -74,64 +73,37 @@ char *semanage_split_on_space(const char *str)
>>  {
>>  	/* as per the man page, these are the isspace() chars */
>>  	const char *seps = "\f\n\r\t\v ";
>> -	size_t slen = strlen(seps);
>> -	size_t off = 0, rside_len = 0;
>> -	char *retval = NULL;
>> -	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
>> +	size_t off = 0;
>>  
>>  	if (!str)
>> -		goto done;
>> -	if (!(ustr = ustr_dup_cstr(str)))
>> -		goto done;
>> -	temp =
>> -	    ustr_split_spn_chrs(ustr, &off, seps, slen, USTR_NULL,
>> -				USTR_FLAG_SPLIT_DEF);
>> -	if (!temp)
>> -		goto done;
>> -	/* throw away the left hand side */
>> -	ustr_sc_free(&temp);
>> -
>> -	rside_len = ustr_len(ustr) - off;
>> -	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
>> -	if (!temp)
>> -		goto done;
>> -	retval = strdup(ustr_cstr(temp));
>> -	ustr_sc_free(&temp);
>> +		return NULL;
>>  
>> -      done:
>> -	ustr_sc_free(&ustr);
>> -	return retval;
>> +	/* skip one token and the spaces before and after it */
>> +	off = strspn(str, seps);
>> +	off += strcspn(str + off, seps);
>> +	off += strspn(str + off, seps);
>> +	return strdup(str + off);
>>  }
>>  
>>  char *semanage_split(const char *str, const char *delim)
>>  {
>> -	Ustr *ustr = USTR_NULL, *temp = USTR_NULL;
>> -	size_t off = 0, rside_len = 0;
>> -	char *retval = NULL;
>> +	char *retval;
>> +	size_t delim_len;
>>  
>>  	if (!str)
>> -		goto done;
>> +		return NULL;
>>  	if (!delim || !(*delim))
>>  		return semanage_split_on_space(str);
>> -	ustr = ustr_dup_cstr(str);
>> -	temp =
>> -	    ustr_split_cstr(ustr, &off, delim, USTR_NULL,
>> USTR_FLAG_SPLIT_DEF);
>> -	if (!temp)
>> -		goto done;
>> -	/* throw away the left hand side */
>> -	ustr_sc_free(&temp);
>> -
>> -	rside_len = ustr_len(ustr) - off;
>>  
>> -	temp = ustr_dup_subustr(ustr, off + 1, rside_len);
>> -	if (!temp)
>> -		goto done;
>> -	retval = strdup(ustr_cstr(temp));
>> -	ustr_sc_free(&temp);
>> +	retval = strstr(str, delim);
>> +	if (retval == NULL)
>> +		return NULL;
>>  
>> -      done:
>> -	ustr_sc_free(&ustr);
>> -	return retval;
>> +	delim_len = strlen(delim);
>> +	do {
>> +		retval += delim_len;
>> +	} while (!strncmp(retval, delim, delim_len));
> 
> This seems unnecessarily complicated.  Do we expect this to ever be
> called on a string containing repeating instances of the delimeter?

In the current code, semanage_split() is either called with delim=NULL
(in which case the call is "forwarded" to semanage_split_on_space()) or
with delim="=" in libsemanage/src/genhomedircon.c (in calls like path =
semanage_findval(PATH_ETC_USERADD, "HOME", "=");). In this last case, it
seems unlikely that parameter str contains several successive
occurrences of the equal sign.

I will drop this do{}while loop in the second version of this patchset.

Thanks,
Nicolas

[PATCH 5/7] libsemanage: add semanage_str_replace() utility function

[Nicolas Iooss]

This function will be used in the next commit.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/src/utilities.c        | 55 ++++++++++++++++++++++++++++++++++++++
 libsemanage/src/utilities.h        | 10 +++++++
 libsemanage/tests/test_utilities.c | 34 +++++++++++++++++++++++
 3 files changed, 99 insertions(+)

diff --git a/libsemanage/src/utilities.c b/libsemanage/src/utilities.c
index b5b92b085310..0b4ee5e654be 100644
--- a/libsemanage/src/utilities.c
+++ b/libsemanage/src/utilities.c
@@ -235,6 +235,61 @@ void semanage_rtrim(char *str, char trim_to)
 	}
 }
 
+char *semanage_str_replace(const char *search, const char *replace,
+			   const char *src, size_t lim)
+{
+	size_t count = 0, slen, rlen, newsize;
+	char *p, *pres, *result;
+	const char *psrc;
+
+	slen = strlen(search);
+	rlen = strlen(replace);
+
+	/* Do not support empty search strings */
+	if (slen == 0)
+		return NULL;
+
+	/* Count the occurences of search in src and compute the new size */
+	for (p = strstr(src, search); p != NULL; p = strstr(p + slen, search)) {
+		count += 1;
+		if (lim && count >= lim)
+			break;
+	}
+	if (!count)
+		return strdup(src);
+
+	/* Allocate the result string */
+	newsize = strlen(src) + 1 + count * (rlen - slen);
+	result = malloc(newsize);
+	if (!result)
+		return NULL;
+
+	/* Fill the result */
+	psrc = src;
+	pres = result;
+	for (p = strstr(src, search); p != NULL; p = strstr(psrc, search)) {
+		/* Copy the part which has not been modified */
+		if (p != psrc) {
+			size_t length = (size_t)(p - psrc);
+			memcpy(pres, psrc, length);
+			pres += length;
+		}
+		/* Copy the replacement part */
+		if (rlen != 0) {
+			memcpy(pres, replace, rlen);
+			pres += rlen;
+		}
+		psrc = p + slen;
+		count -= 1;
+		if (!count)
+			break;
+	}
+	/* Copy the last part, after doing a sanity check */
+	assert(pres + strlen(psrc) + 1 == result + newsize);
+	strcpy(pres, psrc);
+	return result;
+}
+
 /* list_addafter_controlmem does *NOT* duplicate the data argument
  * use at your own risk, I am building a list out of malloc'd memory and
  * it is only going to get stored into this list, thus when I destroy it
diff --git a/libsemanage/src/utilities.h b/libsemanage/src/utilities.h
index 5fa15efd08d0..f2ff31f0f5b6 100644
--- a/libsemanage/src/utilities.h
+++ b/libsemanage/src/utilities.h
@@ -116,6 +116,16 @@ int semanage_str_count(char *data, char what);
 void semanage_rtrim(char *str, char trim_to);
 
 /**
+ * @param      value being searched for
+ * @param      replacement value that replaces found search values
+ * @param      string being searched and replaced on
+ * @param      maximum number of value occurences (zero for unlimited)
+ * @return     newly-allocated string with the replaced values
+ */
+char *semanage_str_replace(const char *search, const char *replace,
+			   const char *src, size_t lim);
+
+/**
  * @param data    some string
  * @return  modifies the string such that the first whitespace char becomes
  *	    '\0', ending the string.
diff --git a/libsemanage/tests/test_utilities.c b/libsemanage/tests/test_utilities.c
index 27fa70dc2a57..7a83596e1566 100644
--- a/libsemanage/tests/test_utilities.c
+++ b/libsemanage/tests/test_utilities.c
@@ -40,6 +40,7 @@ void test_semanage_split(void);
 void test_semanage_list(void);
 void test_semanage_str_count(void);
 void test_semanage_rtrim(void);
+void test_semanage_str_replace(void);
 void test_semanage_findval(void);
 void test_slurp_file_filter(void);
 
@@ -101,6 +102,10 @@ int semanage_utilities_add_tests(CU_pSuite suite)
 	if (NULL == CU_add_test(suite, "semanage_rtrim", test_semanage_rtrim)) {
 		goto err;
 	}
+	if (NULL == CU_add_test(suite, "semanage_str_replace",
+				test_semanage_str_replace)) {
+		goto err;
+	}
 	if (NULL == CU_add_test(suite, "semanage_findval",
 				test_semanage_findval)) {
 		goto err;
@@ -257,6 +262,35 @@ void test_semanage_rtrim(void)
 	free(str);
 }
 
+void test_semanage_str_replace(void)
+{
+	const char *test_str = "Hello, I am %{USERNAME} and my id is %{USERID}";
+	char *str1, *str2;
+
+	str1 = semanage_str_replace("%{USERNAME}", "root", test_str, 0);
+	CU_ASSERT_STRING_EQUAL(str1, "Hello, I am root and my id is %{USERID}");
+
+	str2 = semanage_str_replace("%{USERID}", "0", str1, 1);
+	CU_ASSERT_STRING_EQUAL(str2, "Hello, I am root and my id is 0");
+	free(str1);
+	free(str2);
+
+	str1 = semanage_str_replace(":(", ";)", "Test :( :) ! :(:(:))(:(", 0);
+	CU_ASSERT_STRING_EQUAL(str1, "Test ;) :) ! ;);):))(;)");
+	free(str1);
+
+	str1 = semanage_str_replace(":(", ";)", "Test :( :) ! :(:(:))(:(", 3);
+	CU_ASSERT_STRING_EQUAL(str1, "Test ;) :) ! ;);):))(:(");
+	free(str1);
+
+	str1 = semanage_str_replace("", "empty search string", "test", 0);
+	CU_ASSERT_EQUAL(str1, NULL);
+
+	str1 = semanage_str_replace("a", "", "abracadabra", 0);
+	CU_ASSERT_STRING_EQUAL(str1, "brcdbr");
+	free(str1);
+}
+
 void test_semanage_findval(void)
 {
 	char *tok;
-- 
2.11.0

Re: [PATCH 5/7] libsemanage: add semanage_str_replace() utility function

[Stephen Smalley]

On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
> This function will be used in the next commit.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
>  libsemanage/src/utilities.c        | 55
> ++++++++++++++++++++++++++++++++++++++
>  libsemanage/src/utilities.h        | 10 +++++++
>  libsemanage/tests/test_utilities.c | 34 +++++++++++++++++++++++
>  3 files changed, 99 insertions(+)
> 
> diff --git a/libsemanage/src/utilities.c
> b/libsemanage/src/utilities.c
> index b5b92b085310..0b4ee5e654be 100644
> --- a/libsemanage/src/utilities.c
> +++ b/libsemanage/src/utilities.c
> @@ -235,6 +235,61 @@ void semanage_rtrim(char *str, char trim_to)
>  	}
>  }
>  
> +char *semanage_str_replace(const char *search, const char *replace,
> +			   const char *src, size_t lim)
> +{
> +	size_t count = 0, slen, rlen, newsize;
> +	char *p, *pres, *result;
> +	const char *psrc;
> +
> +	slen = strlen(search);
> +	rlen = strlen(replace);
> +
> +	/* Do not support empty search strings */
> +	if (slen == 0)
> +		return NULL;
> +
> +	/* Count the occurences of search in src and compute the new
> size */
> +	for (p = strstr(src, search); p != NULL; p = strstr(p +
> slen, search)) {
> +		count += 1;

The usage of += 1 instead of just ++ and -= 1 instead of just -- seems
odd to me, but I see that there are other cases of it in the existing
code.  Personally, I'd favor switching to ++ and -- but I won't reject
the patch just on that basis.

> +		if (lim && count >= lim)
> +			break;
> +	}
> +	if (!count)
> +		return strdup(src);
> +
> +	/* Allocate the result string */
> +	newsize = strlen(src) + 1 + count * (rlen - slen);
> +	result = malloc(newsize);
> +	if (!result)
> +		return NULL;
> +
> +	/* Fill the result */
> +	psrc = src;
> +	pres = result;
> +	for (p = strstr(src, search); p != NULL; p = strstr(psrc,
> search)) {
> +		/* Copy the part which has not been modified */
> +		if (p != psrc) {
> +			size_t length = (size_t)(p - psrc);
> +			memcpy(pres, psrc, length);
> +			pres += length;
> +		}
> +		/* Copy the replacement part */
> +		if (rlen != 0) {
> +			memcpy(pres, replace, rlen);
> +			pres += rlen;
> +		}
> +		psrc = p + slen;
> +		count -= 1;
> +		if (!count)
> +			break;
> +	}
> +	/* Copy the last part, after doing a sanity check */
> +	assert(pres + strlen(psrc) + 1 == result + newsize);
> +	strcpy(pres, psrc);
> +	return result;
> +}
> +
>  /* list_addafter_controlmem does *NOT* duplicate the data argument
>   * use at your own risk, I am building a list out of malloc'd memory
> and
>   * it is only going to get stored into this list, thus when I
> destroy it
> diff --git a/libsemanage/src/utilities.h
> b/libsemanage/src/utilities.h
> index 5fa15efd08d0..f2ff31f0f5b6 100644
> --- a/libsemanage/src/utilities.h
> +++ b/libsemanage/src/utilities.h
> @@ -116,6 +116,16 @@ int semanage_str_count(char *data, char what);
>  void semanage_rtrim(char *str, char trim_to);
>  
>  /**
> + * @param      value being searched for
> + * @param      replacement value that replaces found search values
> + * @param      string being searched and replaced on
> + * @param      maximum number of value occurences (zero for
> unlimited)
> + * @return     newly-allocated string with the replaced values
> + */
> +char *semanage_str_replace(const char *search, const char *replace,
> +			   const char *src, size_t lim);
> +
> +/**
>   * @param data    some string
>   * @return  modifies the string such that the first whitespace char
> becomes
>   *	    '\0', ending the string.
> diff --git a/libsemanage/tests/test_utilities.c
> b/libsemanage/tests/test_utilities.c
> index 27fa70dc2a57..7a83596e1566 100644
> --- a/libsemanage/tests/test_utilities.c
> +++ b/libsemanage/tests/test_utilities.c
> @@ -40,6 +40,7 @@ void test_semanage_split(void);
>  void test_semanage_list(void);
>  void test_semanage_str_count(void);
>  void test_semanage_rtrim(void);
> +void test_semanage_str_replace(void);
>  void test_semanage_findval(void);
>  void test_slurp_file_filter(void);
>  
> @@ -101,6 +102,10 @@ int semanage_utilities_add_tests(CU_pSuite
> suite)
>  	if (NULL == CU_add_test(suite, "semanage_rtrim",
> test_semanage_rtrim)) {
>  		goto err;
>  	}
> +	if (NULL == CU_add_test(suite, "semanage_str_replace",
> +				test_semanage_str_replace)) {
> +		goto err;
> +	}
>  	if (NULL == CU_add_test(suite, "semanage_findval",
>  				test_semanage_findval)) {
>  		goto err;
> @@ -257,6 +262,35 @@ void test_semanage_rtrim(void)
>  	free(str);
>  }
>  
> +void test_semanage_str_replace(void)
> +{
> +	const char *test_str = "Hello, I am %{USERNAME} and my id is
> %{USERID}";
> +	char *str1, *str2;
> +
> +	str1 = semanage_str_replace("%{USERNAME}", "root", test_str,
> 0);
> +	CU_ASSERT_STRING_EQUAL(str1, "Hello, I am root and my id is
> %{USERID}");
> +
> +	str2 = semanage_str_replace("%{USERID}", "0", str1, 1);
> +	CU_ASSERT_STRING_EQUAL(str2, "Hello, I am root and my id is
> 0");
> +	free(str1);
> +	free(str2);
> +
> +	str1 = semanage_str_replace(":(", ";)", "Test :( :) !
> :(:(:))(:(", 0);
> +	CU_ASSERT_STRING_EQUAL(str1, "Test ;) :) ! ;);):))(;)");
> +	free(str1);
> +
> +	str1 = semanage_str_replace(":(", ";)", "Test :( :) !
> :(:(:))(:(", 3);
> +	CU_ASSERT_STRING_EQUAL(str1, "Test ;) :) ! ;);):))(:(");
> +	free(str1);
> +
> +	str1 = semanage_str_replace("", "empty search string",
> "test", 0);
> +	CU_ASSERT_EQUAL(str1, NULL);
> +
> +	str1 = semanage_str_replace("a", "", "abracadabra", 0);
> +	CU_ASSERT_STRING_EQUAL(str1, "brcdbr");
> +	free(str1);
> +}
> +
>  void test_semanage_findval(void)
>  {
>  	char *tok;

[PATCH 6/7] libsemanage: genhomedircon: drop ustr dependency

[Nicolas Iooss]

ustr library uses old (pre-C99) "extern inline" semantic. This makes it
incompatible with recent versions of gcc and clang, which default to
C99 standard. Distributions have shipped patched versions of this
library to fix issues (e.g. Gentoo package uses this patch:
https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/files/ustr-1.0.4-gcc_5-check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0
) but there is no upstream solution to make ustr compatible with C99
standard.

The git tree of ustr (http://www.and.org/ustr/ustr.git) has not been
updated since 2008 and the developer of this project did not reply to
emails.

Therefore update genhomedircon implementation in order to no longer
rely on ustr library.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 libsemanage/src/genhomedircon.c | 137 +++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 71 deletions(-)

diff --git a/libsemanage/src/genhomedircon.c b/libsemanage/src/genhomedircon.c
index 5e9d7224a06e..4b7352c8648a 100644
--- a/libsemanage/src/genhomedircon.c
+++ b/libsemanage/src/genhomedircon.c
@@ -34,7 +34,6 @@
 
 #include "utilities.h"
 #include "genhomedircon.h"
-#include <ustr.h>
 
 #include <assert.h>
 #include <limits.h>
@@ -239,46 +238,35 @@ static int fcontext_matches(const semanage_fcontext_t *fcontext, void *varg)
 {
 	const char *oexpr = semanage_fcontext_get_expr(fcontext);
 	fc_match_handle_t *handp = varg;
-	struct Ustr *expr;
+	char *expr = NULL;
 	regex_t re;
 	int type, retval = -1;
+	size_t len;
 
 	/* Only match ALL or DIR */
 	type = semanage_fcontext_get_type(fcontext);
 	if (type != SEMANAGE_FCONTEXT_ALL && type != SEMANAGE_FCONTEXT_ALL)
 		return 0;
 
-	/* Convert oexpr into a Ustr and anchor it at the beginning */
-	expr = ustr_dup_cstr("^");
-	if (expr == USTR_NULL)
-		goto done;
-	if (!ustr_add_cstr(&expr, oexpr))
-		goto done;
+	len = strlen(oexpr);
 
 	/* Strip off trailing ".+" or ".*" */
-	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
-	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
-		if (!ustr_del(&expr, 2))
-			goto done;
-	}
+	if (len >= 2 && oexpr[len - 2] == '.' && strchr("+*", oexpr[len - 1]))
+		len -= 2;
 
 	/* Strip off trailing "(/.*)?" */
-	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
-		if (!ustr_del(&expr, 6))
-			goto done;
-	}
+	if (len >= 6 && !strncmp(oexpr + len - 6, "(/.*)?", 6))
+		len -= 2;
 
-	if (ustr_cmp_suffix_cstr_eq(expr, "/")) {
-		if (!ustr_del(&expr, 1))
-			goto done;
-	}
+	if (len >= 1 && oexpr[len - 1] == '/')
+		len -= 1;
 
-	/* Append pattern to eat up trailing slashes */
-	if (!ustr_add_cstr(&expr, "/*$"))
-		goto done;
+	/* Anchor oexpr at the beginning and append pattern to eat up trailing slashes */
+	if (asprintf(&expr, "^%.*s/*$", (int)len, oexpr) < 0)
+		return -1;
 
 	/* Check dir against expr */
-	if (regcomp(&re, ustr_cstr(expr), REG_EXTENDED) != 0)
+	if (regcomp(&re, expr, REG_EXTENDED) != 0)
 		goto done;
 	if (regexec(&re, handp->dir, 0, NULL, 0) == 0)
 		handp->matched = 1;
@@ -287,7 +275,7 @@ static int fcontext_matches(const semanage_fcontext_t *fcontext, void *varg)
 	retval = 0;
 
 done:
-	ustr_free(expr);
+	free(expr);
 
 	return retval;
 }
@@ -523,44 +511,51 @@ static semanage_list_t *make_template(genhomedircon_settings_t * s,
 	return template_data;
 }
 
-static Ustr *replace_all(const char *str, const replacement_pair_t * repl)
+static char *replace_all(const char *str, const replacement_pair_t * repl)
 {
-	Ustr *retval = USTR_NULL;
+	char *retval, *retval2;
 	int i;
 
 	if (!str || !repl)
-		goto done;
-	if (!(retval = ustr_dup_cstr(str)))
-		goto done;
+		return NULL;
 
-	for (i = 0; repl[i].search_for; i++) {
-		ustr_replace_cstr(&retval, repl[i].search_for,
-				  repl[i].replace_with, 0);
+	retval = strdup(str);
+	for (i = 0; retval != NULL && repl[i].search_for; i++) {
+		retval2 = semanage_str_replace(repl[i].search_for,
+					       repl[i].replace_with, retval, 0);
+		free(retval);
+		retval = retval2;
 	}
-	if (ustr_enomem(retval))
-		ustr_sc_free(&retval);
-
-      done:
 	return retval;
 }
 
-static const char * extract_context(Ustr *line)
+static const char *extract_context(const char *line)
 {
-	const char whitespace[] = " \t\n";
-	size_t off, len;
-
-	/* check for trailing whitespace */
-	off = ustr_spn_chrs_rev(line, 0, whitespace, strlen(whitespace));
-
-	/* find the length of the last field in line */
-	len = ustr_cspn_chrs_rev(line, off, whitespace, strlen(whitespace));
-
-	if (len == 0)
+	const char whitespace[] = {' ', '\t', '\n'};
+	const char *p = line;
+	size_t off;
+
+	off = strlen(p);
+	p += off;
+	/* consider trailing whitespaces */
+	while (off > 0) {
+		p -= 1;
+		off -= 1;
+		if (!memchr(whitespace, *p, sizeof(whitespace)))
+			break;
+	}
+	if (off == 0)
 		return NULL;
-	return ustr_cstr(line) + ustr_len(line) - (len + off);
+
+	/* find the last field in line */
+	while (off > 0 && !memchr(whitespace, *(p - 1), sizeof(whitespace))) {
+		p -= 1;
+		off -= 1;
+	}
+	return p;
 }
 
-static int check_line(genhomedircon_settings_t * s, Ustr *line)
+static int check_line(genhomedircon_settings_t * s, const char *line)
 {
 	sepol_context_t *ctx_record = NULL;
 	const char *ctx_str;
@@ -584,22 +579,22 @@ static int write_replacements(genhomedircon_settings_t * s, FILE * out,
 			      const semanage_list_t * tpl,
 			      const replacement_pair_t *repl)
 {
-	Ustr *line = USTR_NULL;
+	char *line;
 
 	for (; tpl; tpl = tpl->next) {
 		line = replace_all(tpl->data, repl);
 		if (!line)
 			goto fail;
 		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out))
+			if (fprintf(out, "%s\n", line) < 0)
 				goto fail;
 		}
-		ustr_sc_free(&line);
+		free(line);
 	}
 	return STATUS_SUCCESS;
 
       fail:
-	ustr_sc_free(&line);
+	free(line);
 	return STATUS_ERR;
 }
 
@@ -607,7 +602,7 @@ static int write_contexts(genhomedircon_settings_t *s, FILE *out,
 			  semanage_list_t *tpl, const replacement_pair_t *repl,
 			  const genhomedircon_user_entry_t *user)
 {
-	Ustr *line = USTR_NULL;
+	char *line, *temp;
 	sepol_context_t *context = NULL;
 	char *new_context_str = NULL;
 
@@ -624,10 +619,9 @@ static int write_contexts(genhomedircon_settings_t *s, FILE *out,
 
 		if (strcmp(old_context_str, CONTEXT_NONE) == 0) {
 			if (check_line(s, line) == STATUS_SUCCESS &&
-			    !ustr_io_putfileline(&line, out)) {
+			    fprintf(out, "%s\n", line) < 0) {
 				goto fail;
 			}
-
 			continue;
 		}
 
@@ -657,25 +651,27 @@ static int write_contexts(genhomedircon_settings_t *s, FILE *out,
 			goto fail;
 		}
 
-		if (!ustr_replace_cstr(&line, old_context_str,
-				       new_context_str, 1)) {
+		temp = semanage_str_replace(old_context_str, new_context_str,
+					    line, 1);
+		if (!temp) {
 			goto fail;
 		}
+		free(line);
+		line = temp;
 
 		if (check_line(s, line) == STATUS_SUCCESS) {
-			if (!ustr_io_putfileline(&line, out)) {
+			if (fprintf(out, "%s\n", line) < 0)
 				goto fail;
-			}
 		}
 
-		ustr_sc_free(&line);
+		free(line);
 		sepol_context_free(context);
 		free(new_context_str);
 	}
 
 	return STATUS_SUCCESS;
 fail:
-	ustr_sc_free(&line);
+	free(line);
 	sepol_context_free(context);
 	free(new_context_str);
 	return STATUS_ERR;
@@ -1288,20 +1284,19 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 		}
 
 		for (h = homedirs; h; h = h->next) {
-			Ustr *temp = ustr_dup_cstr(h->data);
+			char *temp = NULL;
 
-			if (!temp || !ustr_add_cstr(&temp, "/" FALLBACK_NAME)) {
-				ustr_sc_free(&temp);
+			if (asprintf(&temp, "%s/%s", h->data, FALLBACK_NAME) < 0) {
 				retval = STATUS_ERR;
 				goto done;
 			}
 
 			free(s->fallback->home);
-			s->fallback->home = (char*) ustr_cstr(temp);
+			s->fallback->home = temp;
 
 			if (write_home_dir_context(s, out, homedir_context_tpl,
 						   s->fallback) != STATUS_SUCCESS) {
-				ustr_sc_free(&temp);
+				free(temp);
 				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
@@ -1309,13 +1304,13 @@ static int write_context_file(genhomedircon_settings_t * s, FILE * out)
 			if (write_home_root_context(s, out,
 						    homeroot_context_tpl,
 						    h->data) != STATUS_SUCCESS) {
-				ustr_sc_free(&temp);
+				free(temp);
 				s->fallback->home = NULL;
 				retval = STATUS_ERR;
 				goto done;
 			}
 
-			ustr_sc_free(&temp);
+			free(temp);
 			s->fallback->home = NULL;
 		}
 	}
-- 
2.11.0

Re: [PATCH 6/7] libsemanage: genhomedircon: drop ustr dependency

[Stephen Smalley]

On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
> ustr library uses old (pre-C99) "extern inline" semantic. This makes
> it
> incompatible with recent versions of gcc and clang, which default to
> C99 standard. Distributions have shipped patched versions of this
> library to fix issues (e.g. Gentoo package uses this patch:
> https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/files/us
> tr-1.0.4-gcc_5-
> check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0
> ) but there is no upstream solution to make ustr compatible with C99
> standard.
> 
> The git tree of ustr (http://www.and.org/ustr/ustr.git) has not been
> updated since 2008 and the developer of this project did not reply to
> emails.
> 
> Therefore update genhomedircon implementation in order to no longer
> rely on ustr library.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
>  libsemanage/src/genhomedircon.c | 137 +++++++++++++++++++-----------
> ----------
>  1 file changed, 66 insertions(+), 71 deletions(-)
> 
> diff --git a/libsemanage/src/genhomedircon.c
> b/libsemanage/src/genhomedircon.c
> index 5e9d7224a06e..4b7352c8648a 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
> @@ -239,46 +238,35 @@ static int fcontext_matches(const
> semanage_fcontext_t *fcontext, void *varg)
>  {
>  	const char *oexpr = semanage_fcontext_get_expr(fcontext);
>  	fc_match_handle_t *handp = varg;
> -	struct Ustr *expr;
> +	char *expr = NULL;
>  	regex_t re;
>  	int type, retval = -1;
> +	size_t len;
>  
>  	/* Only match ALL or DIR */
>  	type = semanage_fcontext_get_type(fcontext);
>  	if (type != SEMANAGE_FCONTEXT_ALL && type !=
> SEMANAGE_FCONTEXT_ALL)
>  		return 0;
>  
> -	/* Convert oexpr into a Ustr and anchor it at the beginning
> */
> -	expr = ustr_dup_cstr("^");
> -	if (expr == USTR_NULL)
> -		goto done;
> -	if (!ustr_add_cstr(&expr, oexpr))
> -		goto done;
> +	len = strlen(oexpr);
>  
>  	/* Strip off trailing ".+" or ".*" */
> -	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
> -	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
> -		if (!ustr_del(&expr, 2))
> -			goto done;
> -	}
> +	if (len >= 2 && oexpr[len - 2] == '.' && strchr("+*",
> oexpr[len - 1]))
> +		len -= 2;
>  
>  	/* Strip off trailing "(/.*)?" */
> -	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
> -		if (!ustr_del(&expr, 6))
> -			goto done;
> -	}
> +	if (len >= 6 && !strncmp(oexpr + len - 6, "(/.*)?", 6))
> +		len -= 2;

Should be len -= 6;
Maybe we should make this code less fragile, e.g. #define the
expressions and use sizeof(X)-1 for the lengths in the strncmp() and -=
statements, wrap it all up with a macro.

> @@ -523,44 +511,51 @@ static semanage_list_t
> *make_template(genhomedircon_settings_t * s,
>  	return template_data;
>  }
>  
> -static Ustr *replace_all(const char *str, const replacement_pair_t *
> repl)
> +static char *replace_all(const char *str, const replacement_pair_t *
> repl)
>  {
> -	Ustr *retval = USTR_NULL;
> +	char *retval, *retval2;
>  	int i;
>  
>  	if (!str || !repl)
> -		goto done;
> -	if (!(retval = ustr_dup_cstr(str)))
> -		goto done;
> +		return NULL;
>  
> -	for (i = 0; repl[i].search_for; i++) {
> -		ustr_replace_cstr(&retval, repl[i].search_for,
> -				  repl[i].replace_with, 0);
> +	retval = strdup(str);
> +	for (i = 0; retval != NULL && repl[i].search_for; i++) {
> +		retval2 = semanage_str_replace(repl[i].search_for,
> +					       repl[i].replace_with,
> retval, 0);
> +		free(retval);
> +		retval = retval2;
>  	}
> -	if (ustr_enomem(retval))
> -		ustr_sc_free(&retval);
> -
> -      done:
>  	return retval;
>  }
>  
> -static const char * extract_context(Ustr *line)
> +static const char *extract_context(const char *line)
>  {
> -	const char whitespace[] = " \t\n";
> -	size_t off, len;
> -
> -	/* check for trailing whitespace */
> -	off = ustr_spn_chrs_rev(line, 0, whitespace,
> strlen(whitespace));
> -
> -	/* find the length of the last field in line */
> -	len = ustr_cspn_chrs_rev(line, off, whitespace,
> strlen(whitespace));
> -
> -	if (len == 0)
> +	const char whitespace[] = {' ', '\t', '\n'};
> +	const char *p = line;
> +	size_t off;
> +
> +	off = strlen(p);
> +	p += off;
> +	/* consider trailing whitespaces */
> +	while (off > 0) {
> +		p -= 1;
> +		off -= 1;
> +		if (!memchr(whitespace, *p, sizeof(whitespace)))

if (!isspace(*p))

> +			break;
> +	}
> +	if (off == 0)
>  		return NULL;
> -	return ustr_cstr(line) + ustr_len(line) - (len + off);
> +
> +	/* find the last field in line */
> +	while (off > 0 && !memchr(whitespace, *(p - 1),
> sizeof(whitespace))) {

while (off > 0 && !isspace(*p))

> +		p -= 1;
> +		off -= 1;
> +	}
> +	return p;
>  }

Re: [PATCH 6/7] libsemanage: genhomedircon: drop ustr dependency

[Nicolas Iooss]

On 19/12/16 19:32, Stephen Smalley wrote:
> On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
>> ustr library uses old (pre-C99) "extern inline" semantic. This makes
>> it
>> incompatible with recent versions of gcc and clang, which default to
>> C99 standard. Distributions have shipped patched versions of this
>> library to fix issues (e.g. Gentoo package uses this patch:
>> https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/files/us
>> tr-1.0.4-gcc_5-
>> check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0
>> ) but there is no upstream solution to make ustr compatible with C99
>> standard.
>>
>> The git tree of ustr (http://www.and.org/ustr/ustr.git) has not been
>> updated since 2008 and the developer of this project did not reply to
>> emails.
>>
>> Therefore update genhomedircon implementation in order to no longer
>> rely on ustr library.
>>
>> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
>> ---
>>  libsemanage/src/genhomedircon.c | 137 +++++++++++++++++++-----------
>> ----------
>>  1 file changed, 66 insertions(+), 71 deletions(-)
>>
>> diff --git a/libsemanage/src/genhomedircon.c
>> b/libsemanage/src/genhomedircon.c
>> index 5e9d7224a06e..4b7352c8648a 100644
>> --- a/libsemanage/src/genhomedircon.c
>> +++ b/libsemanage/src/genhomedircon.c
>> @@ -239,46 +238,35 @@ static int fcontext_matches(const
>> semanage_fcontext_t *fcontext, void *varg)
>>  {
>>  	const char *oexpr = semanage_fcontext_get_expr(fcontext);
>>  	fc_match_handle_t *handp = varg;
>> -	struct Ustr *expr;
>> +	char *expr = NULL;
>>  	regex_t re;
>>  	int type, retval = -1;
>> +	size_t len;
>>  
>>  	/* Only match ALL or DIR */
>>  	type = semanage_fcontext_get_type(fcontext);
>>  	if (type != SEMANAGE_FCONTEXT_ALL && type !=
>> SEMANAGE_FCONTEXT_ALL)
>>  		return 0;
>>  
>> -	/* Convert oexpr into a Ustr and anchor it at the beginning
>> */
>> -	expr = ustr_dup_cstr("^");
>> -	if (expr == USTR_NULL)
>> -		goto done;
>> -	if (!ustr_add_cstr(&expr, oexpr))
>> -		goto done;
>> +	len = strlen(oexpr);
>>  
>>  	/* Strip off trailing ".+" or ".*" */
>> -	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
>> -	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
>> -		if (!ustr_del(&expr, 2))
>> -			goto done;
>> -	}
>> +	if (len >= 2 && oexpr[len - 2] == '.' && strchr("+*",
>> oexpr[len - 1]))
>> +		len -= 2;
>>  
>>  	/* Strip off trailing "(/.*)?" */
>> -	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
>> -		if (!ustr_del(&expr, 6))
>> -			goto done;
>> -	}
>> +	if (len >= 6 && !strncmp(oexpr + len - 6, "(/.*)?", 6))
>> +		len -= 2;
> 
> Should be len -= 6;
> Maybe we should make this code less fragile, e.g. #define the
> expressions and use sizeof(X)-1 for the lengths in the strncmp() and -=
> statements, wrap it all up with a macro.

Thanks for your review. I have modified my patches and will send a v2
once I tested them.
For the new macro in fcontext_matches(), here is the code I am testing:

	/* Define a macro to strip a literal string from the end of oexpr */
#define rstrip_oexpr_len(cstr, cstrlen) \
	do { \
		if (len >= cstrlen && !strncmp(oexpr + len - cstrlen, cstr, cstrlen)) \
			len -= cstrlen; \
	} while (0)
#define rstrip_oexpr(cstr) rstrip_oexpr_len(cstr, sizeof(cstr) - 1)

	rstrip_oexpr(".+");
	rstrip_oexpr(".*");
	rstrip_oexpr("(/.*)?");
	rstrip_oexpr("/");

#undef rstrip_oexpr_len
#undef rstrip_oexpr


What would you think of such an implementation?

Nicolas

Re: [PATCH 6/7] libsemanage: genhomedircon: drop ustr dependency

[Stephen Smalley]

On Wed, 2016-12-21 at 16:15 +0100, Nicolas Iooss wrote:
> On 19/12/16 19:32, Stephen Smalley wrote:
> > 
> > On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
> > > 
> > > ustr library uses old (pre-C99) "extern inline" semantic. This
> > > makes
> > > it
> > > incompatible with recent versions of gcc and clang, which default
> > > to
> > > C99 standard. Distributions have shipped patched versions of this
> > > library to fix issues (e.g. Gentoo package uses this patch:
> > > https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/file
> > > s/us
> > > tr-1.0.4-gcc_5-
> > > check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0
> > > ) but there is no upstream solution to make ustr compatible with
> > > C99
> > > standard.
> > > 
> > > The git tree of ustr (http://www.and.org/ustr/ustr.git) has not
> > > been
> > > updated since 2008 and the developer of this project did not
> > > reply to
> > > emails.
> > > 
> > > Therefore update genhomedircon implementation in order to no
> > > longer
> > > rely on ustr library.
> > > 
> > > Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> > > ---
> > >  libsemanage/src/genhomedircon.c | 137 +++++++++++++++++++-------
> > > ----
> > > ----------
> > >  1 file changed, 66 insertions(+), 71 deletions(-)
> > > 
> > > diff --git a/libsemanage/src/genhomedircon.c
> > > b/libsemanage/src/genhomedircon.c
> > > index 5e9d7224a06e..4b7352c8648a 100644
> > > --- a/libsemanage/src/genhomedircon.c
> > > +++ b/libsemanage/src/genhomedircon.c
> > > @@ -239,46 +238,35 @@ static int fcontext_matches(const
> > > semanage_fcontext_t *fcontext, void *varg)
> > >  {
> > >  	const char *oexpr =
> > > semanage_fcontext_get_expr(fcontext);
> > >  	fc_match_handle_t *handp = varg;
> > > -	struct Ustr *expr;
> > > +	char *expr = NULL;
> > >  	regex_t re;
> > >  	int type, retval = -1;
> > > +	size_t len;
> > >  
> > >  	/* Only match ALL or DIR */
> > >  	type = semanage_fcontext_get_type(fcontext);
> > >  	if (type != SEMANAGE_FCONTEXT_ALL && type !=
> > > SEMANAGE_FCONTEXT_ALL)
> > >  		return 0;
> > >  
> > > -	/* Convert oexpr into a Ustr and anchor it at the
> > > beginning
> > > */
> > > -	expr = ustr_dup_cstr("^");
> > > -	if (expr == USTR_NULL)
> > > -		goto done;
> > > -	if (!ustr_add_cstr(&expr, oexpr))
> > > -		goto done;
> > > +	len = strlen(oexpr);
> > >  
> > >  	/* Strip off trailing ".+" or ".*" */
> > > -	if (ustr_cmp_suffix_cstr_eq(expr, ".+") ||
> > > -	    ustr_cmp_suffix_cstr_eq(expr, ".*")) {
> > > -		if (!ustr_del(&expr, 2))
> > > -			goto done;
> > > -	}
> > > +	if (len >= 2 && oexpr[len - 2] == '.' && strchr("+*",
> > > oexpr[len - 1]))
> > > +		len -= 2;
> > >  
> > >  	/* Strip off trailing "(/.*)?" */
> > > -	if (ustr_cmp_suffix_cstr_eq(expr, "(/.*)?")) {
> > > -		if (!ustr_del(&expr, 6))
> > > -			goto done;
> > > -	}
> > > +	if (len >= 6 && !strncmp(oexpr + len - 6, "(/.*)?", 6))
> > > +		len -= 2;
> > 
> > Should be len -= 6;
> > Maybe we should make this code less fragile, e.g. #define the
> > expressions and use sizeof(X)-1 for the lengths in the strncmp()
> > and -=
> > statements, wrap it all up with a macro.
> 
> Thanks for your review. I have modified my patches and will send a v2
> once I tested them.
> For the new macro in fcontext_matches(), here is the code I am
> testing:
> 
> 	/* Define a macro to strip a literal string from the end of
> oexpr */
> #define rstrip_oexpr_len(cstr, cstrlen) \
> 	do { \
> 		if (len >= cstrlen && !strncmp(oexpr + len - cstrlen,
> cstr, cstrlen)) \
> 			len -= cstrlen; \
> 	} while (0)
> #define rstrip_oexpr(cstr) rstrip_oexpr_len(cstr, sizeof(cstr) - 1)
> 
> 	rstrip_oexpr(".+");
> 	rstrip_oexpr(".*");
> 	rstrip_oexpr("(/.*)?");
> 	rstrip_oexpr("/");
> 
> #undef rstrip_oexpr_len
> #undef rstrip_oexpr
> 
> 
> What would you think of such an implementation?

LGTM

Re: [PATCH 6/7] libsemanage: genhomedircon: drop ustr dependency

[Stephen Smalley]

On Mon, 2016-12-19 at 14:04 +0100, Nicolas Iooss wrote:
> ustr library uses old (pre-C99) "extern inline" semantic. This makes
> it
> incompatible with recent versions of gcc and clang, which default to
> C99 standard. Distributions have shipped patched versions of this
> library to fix issues (e.g. Gentoo package uses this patch:
> https://gitweb.gentoo.org/repo/gentoo.git/tree/dev-libs/ustr/files/us
> tr-1.0.4-gcc_5-
> check.patch?id=7dea6f8820f36bf389e6315044bea7507553bed0
> ) but there is no upstream solution to make ustr compatible with C99
> standard.
> 
> The git tree of ustr (http://www.and.org/ustr/ustr.git) has not been
> updated since 2008 and the developer of this project did not reply to
> emails.
> 
> Therefore update genhomedircon implementation in order to no longer
> rely on ustr library.
> 
> Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
> ---
>  libsemanage/src/genhomedircon.c | 137 +++++++++++++++++++-----------
> ----------
>  1 file changed, 66 insertions(+), 71 deletions(-)
> 
> diff --git a/libsemanage/src/genhomedircon.c
> b/libsemanage/src/genhomedircon.c
> index 5e9d7224a06e..4b7352c8648a 100644
> --- a/libsemanage/src/genhomedircon.c
> +++ b/libsemanage/src/genhomedircon.c
<snip>
> @@ -624,10 +619,9 @@ static int
> write_contexts(genhomedircon_settings_t *s, FILE *out,
>  
>  		if (strcmp(old_context_str, CONTEXT_NONE) == 0) {
>  			if (check_line(s, line) == STATUS_SUCCESS &&
> -			    !ustr_io_putfileline(&line, out)) {
> +			    fprintf(out, "%s\n", line) < 0) {
>  				goto fail;
>  			}
> -
>  			continue;

We will leak the memory referenced by 'line' here.

>  		}
>  

[PATCH 7/7] libsemanage: remove ustr library from Makefiles, README and pkg-config

[Nicolas Iooss]

This library is no longer used by libsemanage.

Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
---
 README                            | 2 +-
 libsemanage/src/Makefile          | 2 +-
 libsemanage/src/libsemanage.pc.in | 2 +-
 libsemanage/tests/Makefile        | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/README b/README
index b58150b1bba3..e6f727648205 100644
--- a/README
+++ b/README
@@ -2,7 +2,7 @@ Please submit all bug reports and patches to selinux@tycho.nsa.gov.
 Subscribe via selinux-join@tycho.nsa.gov.
 
 Build dependencies on Fedora:
-yum install audit-libs-devel bison bzip2-devel dbus-devel dbus-glib-devel flex flex-devel flex-static glib2-devel libcap-devel libcap-ng-devel pam-devel pcre-devel python-devel setools-devel swig ustr-devel xmlto redhat-rpm-config
+yum install audit-libs-devel bison bzip2-devel dbus-devel dbus-glib-devel flex flex-devel flex-static glib2-devel libcap-devel libcap-ng-devel pam-devel pcre-devel python-devel setools-devel swig xmlto redhat-rpm-config
 
 To build and install everything under a private directory, run:
 make DESTDIR=~/obj install install-pywrap
diff --git a/libsemanage/src/Makefile b/libsemanage/src/Makefile
index df5809645a00..92c829be234d 100644
--- a/libsemanage/src/Makefile
+++ b/libsemanage/src/Makefile
@@ -91,7 +91,7 @@ $(LIBA): $(OBJS)
 	$(RANLIB) $@
 
 $(LIBSO): $(LOBJS)
-	$(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $^ -lsepol -laudit -lselinux -lbz2 -lustr -L$(LIBDIR) -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs
+	$(CC) $(CFLAGS) $(LDFLAGS) -shared -o $@ $^ -lsepol -laudit -lselinux -lbz2 -L$(LIBDIR) -Wl,-soname,$(LIBSO),--version-script=libsemanage.map,-z,defs
 	ln -sf $@ $(TARGET)
 
 $(LIBPC): $(LIBPC).in ../VERSION
diff --git a/libsemanage/src/libsemanage.pc.in b/libsemanage/src/libsemanage.pc.in
index 81e1805b0826..d3eaa06299c4 100644
--- a/libsemanage/src/libsemanage.pc.in
+++ b/libsemanage/src/libsemanage.pc.in
@@ -7,7 +7,7 @@ Name: libsemanage
 Description: SELinux management library
 Version: @VERSION@
 URL: http://userspace.selinuxproject.org/
-Requires.private: libselinux libsepol ustr
+Requires.private: libselinux libsepol
 Libs: -L${libdir} -lsemanage
 Libs.private: -lbz2
 Cflags: -I${includedir}
diff --git a/libsemanage/tests/Makefile b/libsemanage/tests/Makefile
index bd6a5fd74906..3fe7d16f3e72 100644
--- a/libsemanage/tests/Makefile
+++ b/libsemanage/tests/Makefile
@@ -12,7 +12,7 @@ LIBS = ../src/libsemanage.a -L$(LIBDIR) -lselinux -lsepol
 EXECUTABLE = libsemanage-tests
 CFLAGS += -g -O0 -Wall -W -Wundef -Wmissing-noreturn -Wmissing-format-attribute -Wno-unused-parameter
 INCLUDE = -I../src -I../include
-LDFLAGS += -lcunit -lustr -lbz2 -laudit
+LDFLAGS += -lcunit -lbz2 -laudit
 OBJECTS = $(SOURCES:.c=.o) 
 
 all: $(EXECUTABLE) 
-- 
2.11.0