From 1bb333cd6711ebe80a1bc3a63298eb882297dcca Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 3 Nov 2015 18:46:02 -0800 Subject: Fix GNU/POSIX basename headers. Including glibc's will result in the user getting the POSIX version of basename always, regardless of when it is included relative to . Prior to this patch, our implementation would result in the one that's included first winning. Bug: http://b/25459151 Change-Id: Id4aaf1670dad317d6bbc05763a84ee87596e8e59 --- libc/include/libgen.h | 17 ++++---- libc/include/string.h | 4 +- tests/Android.mk | 1 + tests/libgen_basename_test.cpp | 89 ++++++++++++++++++++++++++++++++++++++++++ tests/libgen_test.cpp | 24 ------------ 5 files changed, 101 insertions(+), 34 deletions(-) create mode 100644 tests/libgen_basename_test.cpp diff --git a/libc/include/libgen.h b/libc/include/libgen.h index e89328e..4d22d15 100644 --- a/libc/include/libgen.h +++ b/libc/include/libgen.h @@ -32,18 +32,19 @@ #include #include + __BEGIN_DECLS -#if !defined(__bionic_using_gnu_basename) /* - * gets you the GNU basename. - * the POSIX one. - * Note that our "POSIX" one has the wrong argument cv-qualifiers, but doesn't - * modify its input and uses thread-local storage for the result if necessary. + * Including will get you the GNU basename, unless is + * included, either before or after including . + * + * Note that this has the wrong argument cv-qualifiers, but doesn't modify its + * input and uses thread-local storage for the result if necessary. */ -extern char* basename(const char*); -#define __bionic_using_posix_basename -#endif +extern char* __posix_basename(const char*) __RENAME(basename); + +#define basename __posix_basename /* This has the wrong argument cv-qualifiers, but doesn't modify its input and uses thread-local storage for the result if necessary. */ extern char* dirname(const char*); diff --git a/libc/include/string.h b/libc/include/string.h index d32c164..8ceccd5 100644 --- a/libc/include/string.h +++ b/libc/include/string.h @@ -107,18 +107,18 @@ extern size_t strxfrm(char* __restrict, const char* __restrict, size_t); extern int strcoll_l(const char *, const char *, locale_t) __purefunc; extern size_t strxfrm_l(char* __restrict, const char* __restrict, size_t, locale_t); -#if defined(__USE_GNU) && !defined(__bionic_using_posix_basename) +#if defined(__USE_GNU) && !defined(basename) /* * glibc has a basename in that's different to the POSIX one in . * It doesn't modify its argument, and in C++ it's const-correct. */ + #if defined(__cplusplus) extern "C++" char* basename(char*) __RENAME(__gnu_basename) __nonnull((1)); extern "C++" const char* basename(const char*) __RENAME(__gnu_basename) __nonnull((1)); #else extern char* basename(const char*) __RENAME(__gnu_basename) __nonnull((1)); #endif -#define __bionic_using_gnu_basename #endif extern void* __memchr_chk(const void*, int, size_t, size_t); diff --git a/tests/Android.mk b/tests/Android.mk index dc2e410..b994cc3 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -64,6 +64,7 @@ libBionicStandardTests_src_files := \ getcwd_test.cpp \ inttypes_test.cpp \ libc_logging_test.cpp \ + libgen_basename_test.cpp \ libgen_test.cpp \ locale_test.cpp \ malloc_test.cpp \ diff --git a/tests/libgen_basename_test.cpp b/tests/libgen_basename_test.cpp new file mode 100644 index 0000000..55939d1 --- /dev/null +++ b/tests/libgen_basename_test.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2012 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#ifndef _GNU_SOURCE + #define _GNU_SOURCE 1 +#endif + +#include + +#if defined(basename) + #error basename should not be defined at this point +#endif + +static const char* gnu_basename(const char* in) { + return basename(in); +} + +#include + +#if !defined(basename) + #error basename should be defined at this point +#endif + +static char* posix_basename(char* in) { + return basename(in); +} + +#include +#include + +static void __TestGnuBasename(const char* in, const char* expected_out, int line) { + const char* out = gnu_basename(in); + ASSERT_STREQ(expected_out, out) << "(" << line << "): " << in << std::endl; + ASSERT_EQ(0, errno) << "(" << line << "): " << in << std::endl; +} + +static void __TestPosixBasename(const char* in, const char* expected_out, int line) { + char* writable_in = (in != NULL) ? strdup(in) : NULL; + errno = 0; + const char* out = posix_basename(&writable_in[0]); + ASSERT_STREQ(expected_out, out) << "(" << line << "): " << in << std::endl; + ASSERT_EQ(0, errno) << "(" << line << "): " << in << std::endl; + free(writable_in); +} + +#define TestGnuBasename(in, expected) __TestGnuBasename(in, expected, __LINE__) +#define TestPosixBasename(in, expected) __TestPosixBasename(in, expected, __LINE__) + +TEST(libgen_basename, gnu_basename) { + // GNU's basename doesn't accept NULL + // TestGnuBasename(NULL, "."); + TestGnuBasename("", ""); + TestGnuBasename("/usr/lib", "lib"); + TestGnuBasename("/system/bin/sh/", ""); + TestGnuBasename("/usr/", ""); + TestGnuBasename("usr", "usr"); + TestGnuBasename("/", ""); + TestGnuBasename(".", "."); + TestGnuBasename("..", ".."); + TestGnuBasename("///", ""); + TestGnuBasename("//usr//lib//", ""); +} + +TEST(libgen_basename, posix_basename) { + TestPosixBasename(NULL, "."); + TestPosixBasename("", "."); + TestPosixBasename("/usr/lib", "lib"); + TestPosixBasename("/system/bin/sh/", "sh"); + TestPosixBasename("/usr/", "usr"); + TestPosixBasename("usr", "usr"); + TestPosixBasename("/", "/"); + TestPosixBasename(".", "."); + TestPosixBasename("..", ".."); + TestPosixBasename("///", "/"); + TestPosixBasename("//usr//lib//", "lib"); +} diff --git a/tests/libgen_test.cpp b/tests/libgen_test.cpp index e9a5d5c..8a37a3f 100644 --- a/tests/libgen_test.cpp +++ b/tests/libgen_test.cpp @@ -19,15 +19,6 @@ #include #include -static void TestBasename(const char* in, const char* expected_out) { - char* writable_in = (in != NULL) ? strdup(in) : NULL; - errno = 0; - const char* out = basename(&writable_in[0]); - ASSERT_STREQ(expected_out, out) << in; - ASSERT_EQ(0, errno) << in; - free(writable_in); -} - static void TestDirname(const char* in, const char* expected_out) { char* writable_in = (in != NULL) ? strdup(in) : NULL; errno = 0; @@ -37,21 +28,6 @@ static void TestDirname(const char* in, const char* expected_out) { free(writable_in); } -// Do not use basename as the test name, it's defined to another value in glibc -// so leads to a differently named test on host versus target architectures. -TEST(libgen, posix_basename) { - TestBasename(NULL, "."); - TestBasename("", "."); - TestBasename("/usr/lib", "lib"); - TestBasename("/usr/", "usr"); - TestBasename("usr", "usr"); - TestBasename("/", "/"); - TestBasename(".", "."); - TestBasename("..", ".."); - TestBasename("///", "/"); - TestBasename("//usr//lib//", "lib"); -} - TEST(libgen, dirname) { TestDirname(NULL, "."); TestDirname("", "."); -- cgit v1.1