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).

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/867424

Title:
  Oneric: On boot up Firefox always displays the “Well, This Is
  Embarrassing” screen.

To manage notifications about this bug go to:
https://bugs.launchpad.net/firefox/+bug/867424/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to