Re: [PATCH 1/1] http: don't send C or POSIX in Accept-Language

[Junio C Hamano <gitster@pobox.com>]

Justin Tobler <jltobler@gmail.com> writes:

> Looking at `get_preferred_languages()` when NO_GETTEXT is defined, we
> already filter out "C" and "POSIX". So doing this for the LANGUAGE
> environment variable when writing the header also makes sense.

True.  I wonder if it makes sense to do the check in that helper
function, though.  I.e. something like

diff --git c/gettext.c w/gettext.c
index 8d08a61f84..e2e0fe339d 100644
--- c/gettext.c
+++ w/gettext.c
@@ -41,6 +41,16 @@ static const char *locale_charset(void)
 
 static const char *charset;
 
+static const char *filter_out_non_languages(const char *candidate)
+{
+	if (candidate && *candidate &&
+	    strcmp(candidate, "C") &&
+	    strcmp(candidate, "POSIX"))
+		return candidate;
+	else
+		return NULL;
+}
+
 /*
  * Guess the user's preferred languages from the value in LANGUAGE environment
  * variable and LC_MESSAGES locale category if NO_GETTEXT is not defined.
@@ -51,15 +61,13 @@ const char *get_preferred_languages(void)
 {
 	const char *retval;
 
-	retval = getenv("LANGUAGE");
-	if (retval && *retval)
+	retval = filter_out_non_languages(getenv("LANGUAGE"));
+	if (retval)
 		return retval;
 
 #ifndef NO_GETTEXT
-	retval = setlocale(LC_MESSAGES, NULL);
-	if (retval && *retval &&
-		strcmp(retval, "C") &&
-		strcmp(retval, "POSIX"))
+	retval = filter_out_non_languages(setlocale(LC_MESSAGES, NULL));
+	if (retval)
 		return retval;
 #endif
 

In the production code, we should have a comment before that new
helper function that explains why we exclude C and POSIX, if we were
to go that route.

> Not sure if being more strict adds much more value here in practice
> though. So it may be fine to keep it as-is. :)

Yup.  I care more about having a single place that checks using the
same logic, than what that logic exactly is ;-).

Thanks.