Code review comment for lp:~ted/ubuntu-app-launch/xmir-support

Revision history for this message
Ted Gould (ted) wrote :

On Mon, 2015-08-10 at 19:12 +0000, Charles Kerr wrote:

> > +TEST_F(ExecUtil, DesktopNoMir)
>
> This is so similar to the previous test, seems like a good candidate for extract-method

Done, now this test takes forever to compile :-)

> > + if (argc < 3) {
> > + fprintf(stderr, "xmir-helper needs more arguments: xmir-helper $(appid) $(thing to exec)\n");
>
> might append "..." to make varargs explicit

Fixed. r139

> > + return 1;
> > + }
> > +
> > + /* Make nice variables for the things we need */
> > + char * appid = argv[1];
> > + char * xmir = getenv("UBUNTU_APP_LAUNCH_XMIR_PATH");
>
> appid and xmir should both be const
>
> also, g_getenv() returns a const gchar* so this should have warned -- do you have warnings turned off in the tests dir?

Verified that we're -Wall and -Werror. But we're using getenv() here so
it's not returning const. Which turns out to be a good thing where in
that exec() can't take a const array. Eh, stdlib :-(

« Back to merge proposal