Comment 43 for bug 867424

Comment on attachment 8754796
(2/2) - Stop using libgnome and libgnomeui on Linux v4

Review of attachment 8754796:
-----------------------------------------------------------------

Looks good, thanks! There are a few style issues, but the file's style is highly inconsistent anyway. While I'm not that familiar with X11 session management, things seem quite sane according to https://www.x.org/releases/X11R7.6/doc/libSM/SMlib.html.

::: toolkit/xre/nsNativeAppSupportUnix.cpp
@@ +429,5 @@
> +
> + --gArgc;
> +}
> +
> +static void setSMValue(SmPropValue& val, const nsCString& data)

Please capitalize function names and add a newline after the return type declaration in implementations.

Also, this block of code isn't guarded by MOZ_X11. Systems without X11 won't be able to resolve SmPropValue.

@@ +436,5 @@
> + val.length = data.Length();
> +}
> +
> +static void setSMProperty(SmProp& prop, const char* name, const char* type,
> + int numVals, SmPropValue vals[])

Style and guard as described above.

@@ +491,5 @@
> + char *arg = *curarg;
> + if (arg[0] == '-' && arg[1] == '-') {
> + arg += 2;
> + if (!strcmp(arg, "sm-disable")) {
> + RemoveArg(curarg);

Is there any reason we can't just increment argv / decrement argc here instead of shifting the char* pointers in RemoveArg?

@@ +576,5 @@
> + char errbuf[256];
> + mSessionConnection = SmcOpenConnection(nullptr, this, SmProtoMajor,
> + SmProtoMinor, mask, &callbacks,
> + prev_client_id.get(), &client_id,
> + 256, errbuf);

Replace 256 with sizeof(errbuf).