Make sure we use the `C` locale for `regcomp` on macOS. by arthurschreiber · Pull Request #3953 · libgit2/libgit2 · GitHub
Skip to content

Make sure we use the C locale for regcomp on macOS. #3953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carlosmn merged 1 commit into from
Oct 6, 2016
Merged

Make sure we use the C locale for regcomp on macOS. #3953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
6 changes: 3 additions & 3 deletions src/config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ int git_config_iterator_glob_new(git_config_iterator **out, const git_config *cf
iter = git__calloc(1, sizeof(all_iter));
GITERR_CHECK_ALLOC(iter);

if ((result = regcomp(&iter->regex, regexp, REG_EXTENDED)) != 0) {
if ((result = p_regcomp(&iter->regex, regexp, REG_EXTENDED)) != 0) {
giterr_set_regex(&iter->regex, result);
git__free(iter);
return -1;
Expand Down Expand Up @@ -512,7 +512,7 @@ int git_config_backend_foreach_match(
int error = 0;

if (regexp != NULL) {
if ((error = regcomp(&regex, regexp, REG_EXTENDED)) != 0) {
if ((error = p_regcomp(&regex, regexp, REG_EXTENDED)) != 0) {
giterr_set_regex(&regex, error);
regfree(&regex);
return -1;
Expand Down Expand Up @@ -1003,7 +1003,7 @@ int git_config_multivar_iterator_new(git_config_iterator **out, const git_config
goto on_error;

if (regexp != NULL) {
error = regcomp(&iter->regex, regexp, REG_EXTENDED);
error = p_regcomp(&iter->regex, regexp, REG_EXTENDED);
if (error != 0) {
giterr_set_regex(&iter->regex, error);
error = -1;
Expand Down
5 changes: 2 additions & 3 deletions src/config_file.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ static int config_set_multivar(
if ((result = git_config__normalize_name(name, &key)) < 0)
return result;

result = regcomp(&preg, regexp, REG_EXTENDED);
result = p_regcomp(&preg, regexp, REG_EXTENDED);
if (result != 0) {
giterr_set_regex(&preg, result);
result = -1;
Expand Down Expand Up @@ -657,7 +657,7 @@ static int config_delete_multivar(git_config_backend *cfg, const char *name, con

refcounted_strmap_free(map);

result = regcomp(&preg, regexp, REG_EXTENDED);
result = p_regcomp(&preg, regexp, REG_EXTENDED);
if (result != 0) {
giterr_set_regex(&preg, result);
result = -1;
Expand Down Expand Up @@ -1957,4 +1957,3 @@ static int config_write(diskfile_backend *cfg, const char *key, const regex_t *p
git_buf_free(&reader->buffer);
return result;
}

7 changes: 3 additions & 4 deletions src/diff_driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ static int diff_driver_add_patterns(
if (error < 0)
break;

if ((error = regcomp(&pat->re, buf.ptr, regex_flags)) != 0) {
if ((error = p_regcomp(&pat->re, buf.ptr, regex_flags)) != 0) {
/*
* TODO: issue a warning
*/
Expand Down Expand Up @@ -210,7 +210,7 @@ static int git_diff_driver_builtin(
goto done;

if (ddef->words &&
(error = regcomp(
(error = p_regcomp(
&drv->word_pattern, ddef->words, ddef->flags | REG_EXTENDED)))
{
error = giterr_set_regex(&drv->word_pattern, error);
Expand Down Expand Up @@ -314,7 +314,7 @@ static int git_diff_driver_load(
goto done;
if (!ce || !ce->value)
/* no diff.<driver>.wordregex, so just continue */;
else if (!(error = regcomp(&drv->word_pattern, ce->value, REG_EXTENDED)))
else if (!(error = p_regcomp(&drv->word_pattern, ce->value, REG_EXTENDED)))
found_driver = true;
else {
/* TODO: warn about bad regex instead of failure */
Expand Down Expand Up @@ -519,4 +519,3 @@ void git_diff_find_context_clear(git_diff_find_context_payload *payload)
payload->driver = NULL;
}
}

2 changes: 1 addition & 1 deletion src/revparse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ static int build_regex(regex_t *regex, const char *pattern)
return GIT_EINVALIDSPEC;
}

error = regcomp(regex, pattern, REG_EXTENDED);
error = p_regcomp(regex, pattern, REG_EXTENDED);
if (!error)
return 0;

Expand Down
10 changes: 10 additions & 0 deletions src/unix/posix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,14 @@ GIT_INLINE(int) p_futimes(int f, const struct p_timeval t[2])
# define p_futimes futimes
#endif

#ifdef HAVE_REGCOMP_L
#include <xlocale.h>
GIT_INLINE(int) p_regcomp(regex_t *preg, const char *pattern, int cflags)
{
return regcomp_l(preg, pattern, cflags, (locale_t) 0);

@carlosmn carlosmn Oct 5, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If (locale_t) 0 defined somewhere to be special? I've found LC_GLOBAL_LOCALE in the headers which is defined as (locale_t) -1 but not for 0.

@arthurschreiber arthurschreiber Oct 5, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I can't find anything either.

Initially, I created a new locale through newlocale(LC_ALL_MASK, "C", (locale_t) 0);, but I tested using (locale_t) 0 directly and that seems to work as well.

We could change it back to using newlocale and storing that locale somewhere if you think that'd be safer?

@arthurschreiber arthurschreiber Oct 5, 2016

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Apple's macOS source code release:

int
tre_regncomp_l(regex_t *preg, const char *regex, size_t n, int cflags, locale_t loc)
{
  ...

  NORMALIZE_LOCALE(loc);

  ...
}
#define NORMALIZE_LOCALE(x) if ((x) == NULL) { \
    (x) = _c_locale; \
} else if ((x) == LC_GLOBAL_LOCALE) { \
    (x) = &__global_locale; \
}

So we should be fine passing (locale_t) 0 (or even just NULL) here. Do you want me to change anything here?

@carlosmn carlosmn Oct 5, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newlocale does mention LC_GLOBAL_LOCALE as not being a valid input as the base, but (locale_t) 0 is, so I suppose it's fine to leave it with 0.

}
#else
# define p_regcomp regcomp
#endif

#endif
3 changes: 3 additions & 0 deletions src/win32/posix.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,4 +57,7 @@ extern int p_lstat_posixly(const char *filename, struct stat *buf);
extern struct tm * p_localtime_r(const time_t *timer, struct tm *result);
extern struct tm * p_gmtime_r(const time_t *timer, struct tm *result);

/* Use the bundled regcomp */
#define p_regcomp regcomp

#endif
27 changes: 26 additions & 1 deletion tests/core/posix.c
View file
Open in desktop