Created attachment 218323 [details] [review]
win32: fix g_get_environ()

The current code create the strv array incorrectly, it is too big and
leaves invalid holes. This may result in crashes when freeing the
returned value.

Created attachment 218324 [details] [review]
win32: fix tests/environment

On Windows, getenv() returns NULL for empty variables.

Comment on attachment 218323 [details] [review]
win32: fix g_get_environ()

>+  for (n = 0, i = 0; strings[n]; n += wcslen (strings + n) + 1, i++);

Put the closing ";" on a separate line, to make it clear that the loop is intentionally empty. (Yeah, the existing code was bad style.) Better yet, reword to make the loop body non-empty.

Otherwise seems right.

Comment on attachment 218324 [details] [review]
win32: fix tests/environment

>+ * references to other environment variables, they are expanded. On
>+ * Windows, the return value is %NULL if the variable is empty.

It seems like this is probably an artifact of how the G_OS_WIN32 g_getenv() behaves; in the "len == 0" case, it needs to check if GetLastError() returns ERROR_ENVVAR_NOT_FOUND; if not, then it means the variable exists but is 0-length.

Created attachment 218430 [details] [review]
win32: fix g_get_environ()

The current code create the strv array incorrectly, it is too big and
leaves invalid holes. This may result in crashes when freeing the
returned value.

Created attachment 218431 [details] [review]
win32: g_getenv() should return "" if variable exists and empty

On Windows, GetEnvironmentVariable() returns 0 for empty variables.
Checking GetLastError() == ERROR_ENVVAR_NOT_FOUND helps make a
difference between a variable that does not exist or an empty one
which should return "".

Review of attachment 218431 [details] [review]:

::: glib/genviron.c
@@ +469,3 @@
+      if (GetLastError () == ERROR_ENVVAR_NOT_FOUND)
+        return NULL;
+      value = "";

eh, that's bad, fixing

Created attachment 218432 [details] [review]
win32: g_getenv() should return "" if variable exists and empty

On Windows, GetEnvironmentVariable() returns 0 for empty variables.
Checking GetLastError() == ERROR_ENVVAR_NOT_FOUND helps make a
difference between a variable that does not exist or an empty one
which should return "".

Comment on attachment 218432 [details] [review]
win32: g_getenv() should return "" if variable exists and empty

>+      quark = g_quark_from_static_string ("");
>+      return g_quark_to_string (quark);

you don't need to do that; you can just: return "";

ok to commit like that

Attachment 218430 [details] pushed as 6007a4b - win32: fix g_get_environ()
Attachment 218432 [details] pushed as bfbfbec - win32: g_getenv() should return "" if variable exists and empty