[v7,1/3] eal/windows: move fnmatch function to header file

Message ID 20230731153902.2350026-2-bruce.richardson@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Split logging functionality out of EAL |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Bruce Richardson July 31, 2023, 3:39 p.m. UTC
  To allow the fnmatch function to be shared between libraries, without
having to export it into the public namespace (since it's not prefixed
with "rte"), we can convert fnmatch.c to replace fnmatch.h. This allows
fnmatch function to be static and limited in scope to the current file,
preventing duplicate definitions if it is used by two libraries, while
also not requiring export for sharing.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
---
 lib/eal/windows/fnmatch.c         | 172 -----------------------------
 lib/eal/windows/include/fnmatch.h | 175 +++++++++++++++++++++++++++---
 lib/eal/windows/meson.build       |   1 -
 3 files changed, 162 insertions(+), 186 deletions(-)
 delete mode 100644 lib/eal/windows/fnmatch.c
  

Comments

David Marchand Aug. 9, 2023, 11:18 a.m. UTC | #1
On Mon, Jul 31, 2023 at 5:39 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> To allow the fnmatch function to be shared between libraries, without
> having to export it into the public namespace (since it's not prefixed
> with "rte"), we can convert fnmatch.c to replace fnmatch.h. This allows
> fnmatch function to be static and limited in scope to the current file,
> preventing duplicate definitions if it is used by two libraries, while
> also not requiring export for sharing.

Overall, it lgtm.

I am surprised those 3 static symbols (see below) do not require being
marked "inline" (to avoid "unused symbols" warnings).
The CI looks ok, so probably I am just paranoid.

I have also a comment on sccsid, see below.


>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---

[snip]

> diff --git a/lib/eal/windows/include/fnmatch.h b/lib/eal/windows/include/fnmatch.h
> index c6b226bd5d..fbf1eef21c 100644
> --- a/lib/eal/windows/include/fnmatch.h
> +++ b/lib/eal/windows/include/fnmatch.h
> @@ -1,20 +1,25 @@
>  /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright(c) 2019 Intel Corporation
> + * Copyright (c) 1989, 1993, 1994
> + *     The Regents of the University of California.  All rights reserved.
> + *
> + * This code is derived from software contributed to Berkeley by
> + * Guido van Rossum.
>   */
> -
>  #ifndef _FNMATCH_H_
>  #define _FNMATCH_H_
>
> -/**
> - * This file is required to support the common code in eal_common_log.c
> - * as Microsoft libc does not contain fnmatch.h. This may be removed in
> - * future releases.
> +#if defined(LIBC_SCCS) && !defined(lint)
> +static const char sccsid[] = "@(#)fnmatch.c    8.2 (Berkeley) 4/16/94";
> +#endif /* LIBC_SCCS and not lint */

Strange to keep this (what looks to be a canary) symbol in a header file.

[snip]

> @@ -25,6 +30,10 @@ extern "C" {
>  #define FNM_CASEFOLD 0x10
>  #define FNM_PREFIX_DIRS 0x20
>
> +#define FNM_EOS        '\0'
> +
> +static const char *fnm_rangematch(const char *, char, int);
> +
>  /**
>   * This function is used for searching a given string source
>   * with the given regular expression pattern.

[snip]


> @@ -41,10 +50,150 @@ extern "C" {
>   * @return
>   *     if the pattern is found then return 0 or else FNM_NOMATCH
>   */
> -int fnmatch(const char *pattern, const char *string, int flags);
> +static int
> +fnmatch(const char *pattern, const char *string, int flags)
> +{
> +       const char *stringstart;
> +       char c, test;
  
Bruce Richardson Aug. 9, 2023, 12:35 p.m. UTC | #2
On Wed, Aug 09, 2023 at 01:18:56PM +0200, David Marchand wrote:
> On Mon, Jul 31, 2023 at 5:39 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > To allow the fnmatch function to be shared between libraries, without
> > having to export it into the public namespace (since it's not prefixed
> > with "rte"), we can convert fnmatch.c to replace fnmatch.h. This allows
> > fnmatch function to be static and limited in scope to the current file,
> > preventing duplicate definitions if it is used by two libraries, while
> > also not requiring export for sharing.
> 
> Overall, it lgtm.
> 
> I am surprised those 3 static symbols (see below) do not require being
> marked "inline" (to avoid "unused symbols" warnings).
> The CI looks ok, so probably I am just paranoid.

Only the functions should need the inline, and I suspect that we don't get
any warnings, since the only files including the header always use the
functions. Will mark them as inline just in case in next version.

> 
> I have also a comment on sccsid, see below.
> 
> 
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> > ---
> 
> [snip]
> 
> > diff --git a/lib/eal/windows/include/fnmatch.h b/lib/eal/windows/include/fnmatch.h
> > index c6b226bd5d..fbf1eef21c 100644
> > --- a/lib/eal/windows/include/fnmatch.h
> > +++ b/lib/eal/windows/include/fnmatch.h
> > @@ -1,20 +1,25 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright(c) 2019 Intel Corporation
> > + * Copyright (c) 1989, 1993, 1994
> > + *     The Regents of the University of California.  All rights reserved.
> > + *
> > + * This code is derived from software contributed to Berkeley by
> > + * Guido van Rossum.
> >   */
> > -
> >  #ifndef _FNMATCH_H_
> >  #define _FNMATCH_H_
> >
> > -/**
> > - * This file is required to support the common code in eal_common_log.c
> > - * as Microsoft libc does not contain fnmatch.h. This may be removed in
> > - * future releases.
> > +#if defined(LIBC_SCCS) && !defined(lint)
> > +static const char sccsid[] = "@(#)fnmatch.c    8.2 (Berkeley) 4/16/94";
> > +#endif /* LIBC_SCCS and not lint */
> 
> Strange to keep this (what looks to be a canary) symbol in a header file.
> 
> [snip]
> 

Yeah, missed that, can probably be removed.

> > @@ -25,6 +30,10 @@ extern "C" {
> >  #define FNM_CASEFOLD 0x10
> >  #define FNM_PREFIX_DIRS 0x20
> >
> > +#define FNM_EOS        '\0'
> > +
> > +static const char *fnm_rangematch(const char *, char, int);
> > +
> >  /**
> >   * This function is used for searching a given string source
> >   * with the given regular expression pattern.
> 
> [snip]
> 
> 
> > @@ -41,10 +50,150 @@ extern "C" {
> >   * @return
> >   *     if the pattern is found then return 0 or else FNM_NOMATCH
> >   */
> > -int fnmatch(const char *pattern, const char *string, int flags);
> > +static int
> > +fnmatch(const char *pattern, const char *string, int flags)
> > +{
> > +       const char *stringstart;
> > +       char c, test;
> 
> 
> -- 
> David Marchand
>
  

Patch

diff --git a/lib/eal/windows/fnmatch.c b/lib/eal/windows/fnmatch.c
deleted file mode 100644
index f622bf54c5..0000000000
--- a/lib/eal/windows/fnmatch.c
+++ /dev/null
@@ -1,172 +0,0 @@ 
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright (c) 1989, 1993, 1994
- *	The Regents of the University of California.  All rights reserved.
- *
- * This code is derived from software contributed to Berkeley by
- * Guido van Rossum.
- */
-
-#if defined(LIBC_SCCS) && !defined(lint)
-static const char sccsid[] = "@(#)fnmatch.c	8.2 (Berkeley) 4/16/94";
-#endif /* LIBC_SCCS and not lint */
-
-/*
- * Function fnmatch() as specified in POSIX 1003.2-1992, section B.6.
- * Compares a filename or pathname to a pattern.
- */
-
-#include <ctype.h>
-#include <string.h>
-#include <stdio.h>
-
-#include "fnmatch.h"
-
-#define EOS	'\0'
-
-static const char *rangematch(const char *, char, int);
-
-int
-fnmatch(const char *pattern, const char *string, int flags)
-{
-	const char *stringstart;
-	char c, test;
-
-	for (stringstart = string;;)
-		switch (c = *pattern++) {
-		case EOS:
-			if ((flags & FNM_LEADING_DIR) && *string == '/')
-				return (0);
-			return (*string == EOS ? 0 : FNM_NOMATCH);
-		case '?':
-			if (*string == EOS)
-				return (FNM_NOMATCH);
-			if (*string == '/' && (flags & FNM_PATHNAME))
-				return (FNM_NOMATCH);
-			if (*string == '.' && (flags & FNM_PERIOD) &&
-			    (string == stringstart ||
-			    ((flags & FNM_PATHNAME) && *(string - 1) == '/')))
-				return (FNM_NOMATCH);
-			++string;
-			break;
-		case '*':
-			c = *pattern;
-			/* Collapse multiple stars. */
-			while (c == '*')
-				c = *++pattern;
-
-			if (*string == '.' && (flags & FNM_PERIOD) &&
-			    (string == stringstart ||
-			    ((flags & FNM_PATHNAME) && *(string - 1) == '/')))
-				return (FNM_NOMATCH);
-
-			/* Optimize for pattern with * at end or before /. */
-			if (c == EOS)
-				if (flags & FNM_PATHNAME)
-					return ((flags & FNM_LEADING_DIR) ||
-					    strchr(string, '/') == NULL ?
-					    0 : FNM_NOMATCH);
-				else
-					return (0);
-			else if (c == '/' && flags & FNM_PATHNAME) {
-				string = strchr(string, '/');
-				if (string == NULL)
-					return (FNM_NOMATCH);
-				break;
-			}
-
-			/* General case, use recursion. */
-			while ((test = *string) != EOS) {
-				if (!fnmatch(pattern, string,
-					flags & ~FNM_PERIOD))
-					return (0);
-				if (test == '/' && flags & FNM_PATHNAME)
-					break;
-				++string;
-			}
-			return (FNM_NOMATCH);
-		case '[':
-			if (*string == EOS)
-				return (FNM_NOMATCH);
-			if (*string == '/' && flags & FNM_PATHNAME)
-				return (FNM_NOMATCH);
-			pattern = rangematch(pattern, *string, flags);
-			if (pattern == NULL)
-				return (FNM_NOMATCH);
-			++string;
-			break;
-		case '\\':
-			if (!(flags & FNM_NOESCAPE)) {
-				c = *pattern++;
-				if (c == EOS) {
-					c = '\\';
-					--pattern;
-				}
-			}
-			/* FALLTHROUGH */
-		default:
-			if (c == *string)
-				;
-			else if ((flags & FNM_CASEFOLD) &&
-				 (tolower((unsigned char)c) ==
-				  tolower((unsigned char)*string)))
-				;
-			else if ((flags & FNM_PREFIX_DIRS) && *string == EOS &&
-			     ((c == '/' && string != stringstart) ||
-			     (string == stringstart+1 && *stringstart == '/')))
-				return (0);
-			else
-				return (FNM_NOMATCH);
-			string++;
-			break;
-		}
-	/* NOTREACHED */
-}
-
-static const char *
-rangematch(const char *pattern, char test, int flags)
-{
-	int negate, ok;
-	char c, c2;
-
-	/*
-	 * A bracket expression starting with an unquoted circumflex
-	 * character produces unspecified results (IEEE 1003.2-1992,
-	 * 3.13.2).  This implementation treats it like '!', for
-	 * consistency with the regular expression syntax.
-	 * J.T. Conklin (conklin@ngai.kaleida.com)
-	 */
-	negate = (*pattern == '!' || *pattern == '^');
-	if (negate)
-		++pattern;
-
-	if (flags & FNM_CASEFOLD)
-		test = tolower((unsigned char)test);
-
-	for (ok = 0; (c = *pattern++) != ']';) {
-		if (c == '\\' && !(flags & FNM_NOESCAPE))
-			c = *pattern++;
-		if (c == EOS)
-			return (NULL);
-
-		if (flags & FNM_CASEFOLD)
-			c = tolower((unsigned char)c);
-
-		c2 = *(pattern + 1);
-		if (*pattern == '-' && c2 != EOS && c2 != ']') {
-			pattern += 2;
-			if (c2 == '\\' && !(flags & FNM_NOESCAPE))
-				c2 = *pattern++;
-			if (c2 == EOS)
-				return (NULL);
-
-			if (flags & FNM_CASEFOLD)
-				c2 = tolower((unsigned char)c2);
-
-			if ((unsigned char)c <= (unsigned char)test &&
-			    (unsigned char)test <= (unsigned char)c2)
-				ok = 1;
-		} else if (c == test)
-			ok = 1;
-	}
-	return (ok == negate ? NULL : pattern);
-}
diff --git a/lib/eal/windows/include/fnmatch.h b/lib/eal/windows/include/fnmatch.h
index c6b226bd5d..fbf1eef21c 100644
--- a/lib/eal/windows/include/fnmatch.h
+++ b/lib/eal/windows/include/fnmatch.h
@@ -1,20 +1,25 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
- * Copyright(c) 2019 Intel Corporation
+ * Copyright (c) 1989, 1993, 1994
+ *	The Regents of the University of California.  All rights reserved.
+ *
+ * This code is derived from software contributed to Berkeley by
+ * Guido van Rossum.
  */
-
 #ifndef _FNMATCH_H_
 #define _FNMATCH_H_
 
-/**
- * This file is required to support the common code in eal_common_log.c
- * as Microsoft libc does not contain fnmatch.h. This may be removed in
- * future releases.
+#if defined(LIBC_SCCS) && !defined(lint)
+static const char sccsid[] = "@(#)fnmatch.c	8.2 (Berkeley) 4/16/94";
+#endif /* LIBC_SCCS and not lint */
+
+/*
+ * Function fnmatch() as specified in POSIX 1003.2-1992, section B.6.
+ * Compares a filename or pathname to a pattern.
  */
-#ifdef __cplusplus
-extern "C" {
-#endif
 
-#include <rte_common.h>
+#include <ctype.h>
+#include <string.h>
+#include <stdio.h>
 
 #define FNM_NOMATCH 1
 
@@ -25,6 +30,10 @@  extern "C" {
 #define FNM_CASEFOLD 0x10
 #define FNM_PREFIX_DIRS 0x20
 
+#define FNM_EOS	'\0'
+
+static const char *fnm_rangematch(const char *, char, int);
+
 /**
  * This function is used for searching a given string source
  * with the given regular expression pattern.
@@ -41,10 +50,150 @@  extern "C" {
  * @return
  *	if the pattern is found then return 0 or else FNM_NOMATCH
  */
-int fnmatch(const char *pattern, const char *string, int flags);
+static int
+fnmatch(const char *pattern, const char *string, int flags)
+{
+	const char *stringstart;
+	char c, test;
+
+	for (stringstart = string;;)
+		switch (c = *pattern++) {
+		case FNM_EOS:
+			if ((flags & FNM_LEADING_DIR) && *string == '/')
+				return (0);
+			return (*string == FNM_EOS ? 0 : FNM_NOMATCH);
+		case '?':
+			if (*string == FNM_EOS)
+				return (FNM_NOMATCH);
+			if (*string == '/' && (flags & FNM_PATHNAME))
+				return (FNM_NOMATCH);
+			if (*string == '.' && (flags & FNM_PERIOD) &&
+			    (string == stringstart ||
+			    ((flags & FNM_PATHNAME) && *(string - 1) == '/')))
+				return (FNM_NOMATCH);
+			++string;
+			break;
+		case '*':
+			c = *pattern;
+			/* Collapse multiple stars. */
+			while (c == '*')
+				c = *++pattern;
+
+			if (*string == '.' && (flags & FNM_PERIOD) &&
+			    (string == stringstart ||
+			    ((flags & FNM_PATHNAME) && *(string - 1) == '/')))
+				return (FNM_NOMATCH);
+
+			/* Optimize for pattern with * at end or before /. */
+			if (c == FNM_EOS)
+				if (flags & FNM_PATHNAME)
+					return ((flags & FNM_LEADING_DIR) ||
+					    strchr(string, '/') == NULL ?
+					    0 : FNM_NOMATCH);
+				else
+					return (0);
+			else if (c == '/' && flags & FNM_PATHNAME) {
+				string = strchr(string, '/');
+				if (string == NULL)
+					return (FNM_NOMATCH);
+				break;
+			}
+
+			/* General case, use recursion. */
+			while ((test = *string) != FNM_EOS) {
+				if (!fnmatch(pattern, string,
+					flags & ~FNM_PERIOD))
+					return (0);
+				if (test == '/' && flags & FNM_PATHNAME)
+					break;
+				++string;
+			}
+			return (FNM_NOMATCH);
+		case '[':
+			if (*string == FNM_EOS)
+				return (FNM_NOMATCH);
+			if (*string == '/' && flags & FNM_PATHNAME)
+				return (FNM_NOMATCH);
+			pattern = fnm_rangematch(pattern, *string, flags);
+			if (pattern == NULL)
+				return (FNM_NOMATCH);
+			++string;
+			break;
+		case '\\':
+			if (!(flags & FNM_NOESCAPE)) {
+				c = *pattern++;
+				if (c == FNM_EOS) {
+					c = '\\';
+					--pattern;
+				}
+			}
+			/* FALLTHROUGH */
+		default:
+			if (c == *string)
+				;
+			else if ((flags & FNM_CASEFOLD) &&
+				 (tolower((unsigned char)c) ==
+				  tolower((unsigned char)*string)))
+				;
+			else if ((flags & FNM_PREFIX_DIRS) && *string == FNM_EOS &&
+			     ((c == '/' && string != stringstart) ||
+			     (string == stringstart+1 && *stringstart == '/')))
+				return (0);
+			else
+				return (FNM_NOMATCH);
+			string++;
+			break;
+		}
+	/* NOTREACHED */
+}
+
+static const char *
+fnm_rangematch(const char *pattern, char test, int flags)
+{
+	int negate, ok;
+	char c, c2;
+
+	/*
+	 * A bracket expression starting with an unquoted circumflex
+	 * character produces unspecified results (IEEE 1003.2-1992,
+	 * 3.13.2).  This implementation treats it like '!', for
+	 * consistency with the regular expression syntax.
+	 * J.T. Conklin (conklin@ngai.kaleida.com)
+	 */
+	negate = (*pattern == '!' || *pattern == '^');
+	if (negate)
+		++pattern;
+
+	if (flags & FNM_CASEFOLD)
+		test = tolower((unsigned char)test);
+
+	for (ok = 0; (c = *pattern++) != ']';) {
+		if (c == '\\' && !(flags & FNM_NOESCAPE))
+			c = *pattern++;
+		if (c == FNM_EOS)
+			return (NULL);
+
+		if (flags & FNM_CASEFOLD)
+			c = tolower((unsigned char)c);
+
+		c2 = *(pattern + 1);
+		if (*pattern == '-' && c2 != FNM_EOS && c2 != ']') {
+			pattern += 2;
+			if (c2 == '\\' && !(flags & FNM_NOESCAPE))
+				c2 = *pattern++;
+			if (c2 == FNM_EOS)
+				return (NULL);
+
+			if (flags & FNM_CASEFOLD)
+				c2 = tolower((unsigned char)c2);
 
-#ifdef __cplusplus
+			if ((unsigned char)c <= (unsigned char)test &&
+			    (unsigned char)test <= (unsigned char)c2)
+				ok = 1;
+		} else if (c == test)
+			ok = 1;
+	}
+	return (ok == negate ? NULL : pattern);
 }
-#endif
 
 #endif /* _FNMATCH_H_ */
diff --git a/lib/eal/windows/meson.build b/lib/eal/windows/meson.build
index 845e406ca1..e4b2427610 100644
--- a/lib/eal/windows/meson.build
+++ b/lib/eal/windows/meson.build
@@ -18,7 +18,6 @@  sources += files(
         'eal_mp.c',
         'eal_thread.c',
         'eal_timer.c',
-        'fnmatch.c',
         'getopt.c',
         'rte_thread.c',
 )