[PATCH v2] Move polkit to mozjs38

[Jeremy Linton]

Hi,


On 04/04/2017 03:05 PM, Miloslav Trmac wrote:
> Hello,
> 2017-04-04 21:10 GMT+02:00 Jeremy Linton <jeremy.linton at arm.com
> <mailto:jeremy.linton at arm.com>>:
>
>     Update polkit to use a more recent version of the mozjs library.
>
> …
>
>       v1->v2: Switch back to using initjs.j rather than init.js
>
>
> Another not-really-a-review:
>     Mirek
>
>
>     @@ -1209,10 +1207,13 @@
>     polkit_backend_js_authority_check_authorization_sync
>     (PolkitBackendInteractiveAu
>            goto out;
>          }
>
>     +  argv[0].setObject(argv0.toObject());
>     +  argv[1].setObject(argv1.toObject());
>     +
>        if (!call_js_function_with_runaway_killer (authority,
>                                                   "_runRules",
>     -                                             G_N_ELEMENTS (argv),
>     -                                             argv,
>     +                                             //
>      G_N_ELEMENTS (argv),
>     +                                             &argv,
>                                                   &rval))
>          {
>            polkit_backend_authority_log (POLKIT_BACKEND_AUTHORITY
>     (authority),
>     @@ -1220,22 +1221,17 @@
>     polkit_backend_js_authority_check_authorization_sync
>     (PolkitBackendInteractiveAu
>            goto out;
>          }
>
>     -  if (JSVAL_IS_NULL (rval))
>     -    {
>     -      /* this fine, means there was no match, use implicit
>     authorizations */
>     -      good = TRUE;
>     -      goto out;
>     -    }
>     -
>     -  if (!JSVAL_IS_STRING (rval))
>     +  if (!rval.isString())
>
>
>
> Removing this null check means that when polkit._runRules returns null,
> “ret” is set to POLKIT_IMPLICIT_AUTHORIZATION_NOT_AUTHORIZED instead of
> “implicit”.  To reproduce, on an unmodified Fedora box, interactively
> logged in, compare the result of
>
>     pkcheck -p $$ -a org.fedoraproject.FirewallD1.info
>     <http://org.fedoraproject.FirewallD1.info>
>
>
> (and perhaps observe the “WARNING **: Expected a string” messages on
> stdout when polkitd is running interactively.)

Thanks for taking a look at this. It seems there should be a matching 
rval.isNull() check. I should probably loose the G_N_ELEMENTS(argv) 
comment as well. And since this is a functional change the unit test 
should probably be checking for it to avoid others making that same mistake.


>     Mirek
>