[v2,2/4] Add a new function child_path.
Commit Message
child_path returns a pointer to the first component in a child path
that comes after a parent path. This does not depend on trying to
stat() the paths since they may describe remote paths but instead
relies on filename parsing. The function requires that the child path
describe a filename that contains at least one component below the
parent path and returns a pointer to the first component.
gdb/ChangeLog:
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/child-path-selftests.c.
* common/pathstuff.c (child_path): New function.
* common/pathstuff.h (child_path): New prototype.
* unittests/child-path-selftests.c: New file.
---
gdb/ChangeLog | 8 ++++
gdb/Makefile.in | 1 +
gdb/common/pathstuff.c | 52 ++++++++++++++++++++++
gdb/common/pathstuff.h | 6 +++
gdb/unittests/child-path-selftests.c | 64 ++++++++++++++++++++++++++++
5 files changed, 131 insertions(+)
create mode 100644 gdb/unittests/child-path-selftests.c
Comments
On 2019-01-28 3:47 p.m., John Baldwin wrote:
> child_path returns a pointer to the first component in a child path
> that comes after a parent path. This does not depend on trying to
> stat() the paths since they may describe remote paths but instead
> relies on filename parsing. The function requires that the child path
> describe a filename that contains at least one component below the
> parent path and returns a pointer to the first component.
>
> gdb/ChangeLog:
>
> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> unittests/child-path-selftests.c.
> * common/pathstuff.c (child_path): New function.
> * common/pathstuff.h (child_path): New prototype.
> * unittests/child-path-selftests.c: New file.
Thanks, this LGTM. Just minor comments below.
> ---
> gdb/ChangeLog | 8 ++++
> gdb/Makefile.in | 1 +
> gdb/common/pathstuff.c | 52 ++++++++++++++++++++++
> gdb/common/pathstuff.h | 6 +++
> gdb/unittests/child-path-selftests.c | 64 ++++++++++++++++++++++++++++
> 5 files changed, 131 insertions(+)
> create mode 100644 gdb/unittests/child-path-selftests.c
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 38d740e440..93a2cebe1c 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,11 @@
> +2019-01-28 John Baldwin <jhb@FreeBSD.org>
> +
> + * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
> + unittests/child-path-selftests.c.
> + * common/pathstuff.c (child_path): New function.
> + * common/pathstuff.h (child_path): New prototype.
> + * unittests/child-path-selftests.c: New file.
> +
> 2019-01-28 John Baldwin <jhb@FreeBSD.org>
>
> * symfile.c (find_separate_debug_file): Look for separate debug
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 72ca855eb0..cec7ad32a4 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -411,6 +411,7 @@ SUBDIR_PYTHON_CFLAGS =
>
> SUBDIR_UNITTESTS_SRCS = \
> unittests/array-view-selftests.c \
> + unittests/child-path-selftests.c \
> unittests/cli-utils-selftests.c \
> unittests/common-utils-selftests.c \
> unittests/copy_bitwise-selftests.c \
> diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
> index 11675303b3..d6beb8faf1 100644
> --- a/gdb/common/pathstuff.c
> +++ b/gdb/common/pathstuff.c
> @@ -147,6 +147,58 @@ gdb_abspath (const char *path)
>
> /* See common/pathstuff.h. */
>
> +const char *
> +child_path (const char *parent, const char *child)
> +{
> + /* The child path must start with the parent path. */
> + size_t parent_len = strlen (parent);
> + if (filename_ncmp (parent, child, parent_len) != 0)
> + return NULL;
> +
> + /* The parent path must be a directory and the child must contain at
> + least one component underneath the parent. */
> + const char *child_component;
> + if (IS_DIR_SEPARATOR (parent[parent_len - 1]))
> + {
> + /* The parent path ends in a directory separator, so it is a
> + directory. The first child component starts after the common
> + prefix. */
> + child_component = child + parent_len;
> + }
> + else
> + {
> + /* The parent path does not end in a directory separator. The
> + first character in the child after the common prefix must be
> + a directory separator.
> +
> + Note that CHILD must hold at least parent_len characters for
> + filename_ncmp to return zero. If the character at parent_len
> + is nul due to CHILD containing the same path as PARENT, the
> + IS_DIR_SEPARATOR check will fail here. */
> + if (!IS_DIR_SEPARATOR (child[parent_len]))
> + return NULL;
> +
> + /* The first child component starts after the separator after the
> + common prefix. */
> + child_component = child + parent_len + 1;
> + }
> +
> + /* The child must contain at least one non-separator character after
> + the parent. */
> + while (*child_component != '\0')
> + {
> + if (IS_DIR_SEPARATOR (*child_component))
> + {
> + child_component++;
> + continue;
> + }
> + return child_component;
This might be simplified (avoid the continue) by reversing the logic, using
this as the loop body:
{
if (!IS_DIR_SEPARATOR (*child_component))
return child_component;
child_component++;
}
> + }
> + return NULL;
> +}
> +
> +/* See common/pathstuff.h. */
> +
> bool
> contains_dir_separator (const char *path)
> {
> diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
> index d43f337550..20a7bdda26 100644
> --- a/gdb/common/pathstuff.h
> +++ b/gdb/common/pathstuff.h
> @@ -48,6 +48,12 @@ extern gdb::unique_xmalloc_ptr<char>
>
> extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
>
> +/* If the path in CHILD is a child of the path in PARENT, return a
> + pointer to the first component in the CHILD's pathname below the
> + PARENT. Otherwise, return NULL. */
> +
> +extern const char *child_path (const char *parent, const char *child);
> +
> /* Return whether PATH contains a directory separator character. */
>
> extern bool contains_dir_separator (const char *path);
> diff --git a/gdb/unittests/child-path-selftests.c b/gdb/unittests/child-path-selftests.c
> new file mode 100644
> index 0000000000..2621eecef6
> --- /dev/null
> +++ b/gdb/unittests/child-path-selftests.c
> @@ -0,0 +1,64 @@
> +/* Self tests for child_path for GDB, the GNU debugger.
> +
> + Copyright (C) 2019 Free Software Foundation, Inc.
> +
> + This file is part of GDB.
> +
> + This program is free software; you can redistribute it and/or modify
> + it under the terms of the GNU General Public License as published by
> + the Free Software Foundation; either version 3 of the License, or
> + (at your option) any later version.
> +
> + This program is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + GNU General Public License for more details.
> +
> + You should have received a copy of the GNU General Public License
> + along with this program. If not, see <http://www.gnu.org/licenses/>. */
> +
> +#include "defs.h"
> +#include "common/pathstuff.h"
> +#include "common/selftest.h"
> +
> +namespace selftests {
> +namespace child_path {
> +
> +/* Verify the result of a single child_path test. */
> +
> +static bool
> +child_path_check (const char *parent, const char *child, const char *expected)
> +{
> + const char *result = ::child_path (parent, child);
> + if (result == NULL || expected == NULL)
> + return result == expected;
> + return strcmp (result, expected) == 0;
> +}
> +
> +/* Test child_path. */
> +
> +static void
> +test ()
> +{
> + SELF_CHECK (child_path_check ("/one", "/two", NULL));
> + SELF_CHECK (child_path_check ("/one", "/one", NULL));
> + SELF_CHECK (child_path_check ("/one", "/one/", NULL));
> + SELF_CHECK (child_path_check ("/one", "/one//", NULL));
> + SELF_CHECK (child_path_check ("/one", "/one/two", "two"));
> + SELF_CHECK (child_path_check ("/one/", "/two", NULL));
> + SELF_CHECK (child_path_check ("/one/", "/one", NULL));
> + SELF_CHECK (child_path_check ("/one/", "/one/", NULL));
> + SELF_CHECK (child_path_check ("/one/", "/one//", NULL));
> + SELF_CHECK (child_path_check ("/one/", "/one/two", "two"));
> +}
Here are a few more edge cases I could think of:
SELF_CHECK (child_path_check ("/one/", "/one//two", "two"));
SELF_CHECK (child_path_check ("/one/", "/one//two/", "two/"));
SELF_CHECK (child_path_check ("/one", "/onetwo", NULL));
SELF_CHECK (child_path_check ("/one", "/onetwo/three", NULL));
> +
> +}
> +}
> +
> +void
> +_initialize_child_path_selftests ()
> +{
> + selftests::register_test ("child_path",
> + selftests::child_path::test);
> +}
> +
>
Simon
On 2019-02-11 9:43 p.m., Simon Marchi wrote:
> On 2019-01-28 3:47 p.m., John Baldwin wrote:
>> child_path returns a pointer to the first component in a child path
>> that comes after a parent path. This does not depend on trying to
>> stat() the paths since they may describe remote paths but instead
>> relies on filename parsing. The function requires that the child path
>> describe a filename that contains at least one component below the
>> parent path and returns a pointer to the first component.
>>
>> gdb/ChangeLog:
>>
>> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
>> unittests/child-path-selftests.c.
>> * common/pathstuff.c (child_path): New function.
>> * common/pathstuff.h (child_path): New prototype.
>> * unittests/child-path-selftests.c: New file.
>
> Thanks, this LGTM. Just minor comments below.
Oh, and maybe name the function is_child_path or child_path_p?
Simon
On 2/11/19 6:46 PM, Simon Marchi wrote:
> On 2019-02-11 9:43 p.m., Simon Marchi wrote:
>> On 2019-01-28 3:47 p.m., John Baldwin wrote:
>>> child_path returns a pointer to the first component in a child path
>>> that comes after a parent path. This does not depend on trying to
>>> stat() the paths since they may describe remote paths but instead
>>> relies on filename parsing. The function requires that the child path
>>> describe a filename that contains at least one component below the
>>> parent path and returns a pointer to the first component.
>>>
>>> gdb/ChangeLog:
>>>
>>> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
>>> unittests/child-path-selftests.c.
>>> * common/pathstuff.c (child_path): New function.
>>> * common/pathstuff.h (child_path): New prototype.
>>> * unittests/child-path-selftests.c: New file.
>>
>> Thanks, this LGTM. Just minor comments below.
>
> Oh, and maybe name the function is_child_path or child_path_p?
I started with that name in an earlier version when it returned a boolean,
but renamed it when I found that I needed it to return the trailing portion
of the child pathname to avoid duplicating logic in the caller. Maybe
"get_child_path" would be better?
On 2019-02-12 11:52, John Baldwin wrote:
> On 2/11/19 6:46 PM, Simon Marchi wrote:
>> On 2019-02-11 9:43 p.m., Simon Marchi wrote:
>>> On 2019-01-28 3:47 p.m., John Baldwin wrote:
>>>> child_path returns a pointer to the first component in a child path
>>>> that comes after a parent path. This does not depend on trying to
>>>> stat() the paths since they may describe remote paths but instead
>>>> relies on filename parsing. The function requires that the child
>>>> path
>>>> describe a filename that contains at least one component below the
>>>> parent path and returns a pointer to the first component.
>>>>
>>>> gdb/ChangeLog:
>>>>
>>>> * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
>>>> unittests/child-path-selftests.c.
>>>> * common/pathstuff.c (child_path): New function.
>>>> * common/pathstuff.h (child_path): New prototype.
>>>> * unittests/child-path-selftests.c: New file.
>>>
>>> Thanks, this LGTM. Just minor comments below.
>>
>> Oh, and maybe name the function is_child_path or child_path_p?
>
> I started with that name in an earlier version when it returned a
> boolean,
> but renamed it when I found that I needed it to return the trailing
> portion
> of the child pathname to avoid duplicating logic in the caller. Maybe
> "get_child_path" would be better?
Ah you're right, it doesn't make sense if the function doesn't return a
bool. What you have is fine with me.
Simon
@@ -1,3 +1,11 @@
+2019-01-28 John Baldwin <jhb@FreeBSD.org>
+
+ * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
+ unittests/child-path-selftests.c.
+ * common/pathstuff.c (child_path): New function.
+ * common/pathstuff.h (child_path): New prototype.
+ * unittests/child-path-selftests.c: New file.
+
2019-01-28 John Baldwin <jhb@FreeBSD.org>
* symfile.c (find_separate_debug_file): Look for separate debug
@@ -411,6 +411,7 @@ SUBDIR_PYTHON_CFLAGS =
SUBDIR_UNITTESTS_SRCS = \
unittests/array-view-selftests.c \
+ unittests/child-path-selftests.c \
unittests/cli-utils-selftests.c \
unittests/common-utils-selftests.c \
unittests/copy_bitwise-selftests.c \
@@ -147,6 +147,58 @@ gdb_abspath (const char *path)
/* See common/pathstuff.h. */
+const char *
+child_path (const char *parent, const char *child)
+{
+ /* The child path must start with the parent path. */
+ size_t parent_len = strlen (parent);
+ if (filename_ncmp (parent, child, parent_len) != 0)
+ return NULL;
+
+ /* The parent path must be a directory and the child must contain at
+ least one component underneath the parent. */
+ const char *child_component;
+ if (IS_DIR_SEPARATOR (parent[parent_len - 1]))
+ {
+ /* The parent path ends in a directory separator, so it is a
+ directory. The first child component starts after the common
+ prefix. */
+ child_component = child + parent_len;
+ }
+ else
+ {
+ /* The parent path does not end in a directory separator. The
+ first character in the child after the common prefix must be
+ a directory separator.
+
+ Note that CHILD must hold at least parent_len characters for
+ filename_ncmp to return zero. If the character at parent_len
+ is nul due to CHILD containing the same path as PARENT, the
+ IS_DIR_SEPARATOR check will fail here. */
+ if (!IS_DIR_SEPARATOR (child[parent_len]))
+ return NULL;
+
+ /* The first child component starts after the separator after the
+ common prefix. */
+ child_component = child + parent_len + 1;
+ }
+
+ /* The child must contain at least one non-separator character after
+ the parent. */
+ while (*child_component != '\0')
+ {
+ if (IS_DIR_SEPARATOR (*child_component))
+ {
+ child_component++;
+ continue;
+ }
+ return child_component;
+ }
+ return NULL;
+}
+
+/* See common/pathstuff.h. */
+
bool
contains_dir_separator (const char *path)
{
@@ -48,6 +48,12 @@ extern gdb::unique_xmalloc_ptr<char>
extern gdb::unique_xmalloc_ptr<char> gdb_abspath (const char *path);
+/* If the path in CHILD is a child of the path in PARENT, return a
+ pointer to the first component in the CHILD's pathname below the
+ PARENT. Otherwise, return NULL. */
+
+extern const char *child_path (const char *parent, const char *child);
+
/* Return whether PATH contains a directory separator character. */
extern bool contains_dir_separator (const char *path);
new file mode 100644
@@ -0,0 +1,64 @@
+/* Self tests for child_path for GDB, the GNU debugger.
+
+ Copyright (C) 2019 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "common/pathstuff.h"
+#include "common/selftest.h"
+
+namespace selftests {
+namespace child_path {
+
+/* Verify the result of a single child_path test. */
+
+static bool
+child_path_check (const char *parent, const char *child, const char *expected)
+{
+ const char *result = ::child_path (parent, child);
+ if (result == NULL || expected == NULL)
+ return result == expected;
+ return strcmp (result, expected) == 0;
+}
+
+/* Test child_path. */
+
+static void
+test ()
+{
+ SELF_CHECK (child_path_check ("/one", "/two", NULL));
+ SELF_CHECK (child_path_check ("/one", "/one", NULL));
+ SELF_CHECK (child_path_check ("/one", "/one/", NULL));
+ SELF_CHECK (child_path_check ("/one", "/one//", NULL));
+ SELF_CHECK (child_path_check ("/one", "/one/two", "two"));
+ SELF_CHECK (child_path_check ("/one/", "/two", NULL));
+ SELF_CHECK (child_path_check ("/one/", "/one", NULL));
+ SELF_CHECK (child_path_check ("/one/", "/one/", NULL));
+ SELF_CHECK (child_path_check ("/one/", "/one//", NULL));
+ SELF_CHECK (child_path_check ("/one/", "/one/two", "two"));
+}
+
+}
+}
+
+void
+_initialize_child_path_selftests ()
+{
+ selftests::register_test ("child_path",
+ selftests::child_path::test);
+}
+