Comment on revision rb1fc9fc7278bf7b0555bebb3cbdbf5b96b5736c7 in gsignond.accounts-sso

[accoun...@googlecode.com]

Comment by alex.kan...@gmail.com:

General Comment:
See
https://code.google.com/p/accounts-sso/source/detail?repo=gsignond&r=223e97eb9de7afd7699d5dadec6e0370617bff57
for reasons.

For more information:
https://code.google.com/p/accounts-sso/source/detail?r=b1fc9fc7278bf7b0555bebb3cbdbf5b96b5736c7&repo=gsignond

[accoun...@googlecode.com]

Comment by jussi.la...@linux.intel.com:

General Comment:
Imran, can you try to reproduce the problem so that we can see where it
comes from?

[accoun...@googlecode.com]

Comment by imran.za...@gmail.com:

Score: Neutral

General Comment:
The check is always good to have; does not harm anything if we have this
check and unfortunately don't have enough time to reproduce the bug as I
don't remember what exactly is it. I would still support this commit as its
always good to check the input arguments before to proceed further..

[accoun...@googlecode.com]

Comment by alex.kan...@gmail.com:

General Comment:

I think Jussi (or someone) was going to implement a proper sanity check for
the list of plugins returned by the plugin loader (e.g. that they are all
non-empty alphanumeric strings). I'll create an issue about it now.

[accoun...@googlecode.com]

Comment by imran.za...@gmail.com:

Score: Negative

General Comment:
Here is the problem i see in the test case log
(test/plugins/pluginproxytest.log) at least if we don't have the check..
why is lib.so being getting loaded?

(gsignond-plugind:32636): gsignond-DEBUG: 9070453.150026
gsignond-plugin-loader.c:60 gsignond_load_plugin_with_filename Loading
plugin ../../src/plugins/.libs/lib.so
(gsignond-plugind:32636): gsignond-DEBUG: 9070453.150109
gsignond-plugin-loader.c:64 gsignond_load_plugin_with_filename Plugin
couldn't be opened: ../../src/plugins/.libs/lib.so: cannot open shared
object file: No such file or directory
(gsignond-plugind:32636): gsignond-DEBUG: 9070453.150157
gsignond-plugin-daemon.c:392 gsignond_plugin_daemon_new failed to load
plugin
(process:32613): gsignond-DEBUG: 9070453.150408
gsignond-plugin-remote.c:545 gsignond_plugin_remote_new '/' object
exported(0x24a0480)
(process:32613): gsignond-DEBUG: 9070453.150466
gsignond-plugin-proxy-factory.c:107 _add_plugins Plugin returned type
property (null), which does not match requested type
(process:32613): gsignond-DEBUG: 9070453.150495
gsignond-plugin-remote.c:129 gsignond_plugin_remote_dispose Send SIGTERM to
Plugind
(process:32613): gsignond-DEBUG: 9070453.150523 gsignond-plugin-remote.c:69
_on_child_down_cb Plugind(0x24a0480) with pid (32636) closed with status
65280
(process:32613): gsignond-DEBUG: 9070453.150554
gsignond-plugin-remote.c:135 gsignond_plugin_remote_dispose Plugind
DESTROYED

[accoun...@googlecode.com]

Comment by alex.kan...@gmail.com:

General Comment:

Because \n at the end of the list of plugins makes gsso think that "" is
one of the available plugins. It's not in itself a problem, because lib.so
file is absent, so attempting to load "" plugin fails as it should. The
proposed sanity check would take care of this.

[accoun...@googlecode.com]

Comment by imran.za...@gmail.com:

Score: Negative

General Comment:

Yes at least that check takes care of this case :)

[accoun...@googlecode.com]

Comment by alex.kan...@gmail.com:

General Comment:

Yes, but we need a more general check.

[accoun...@googlecode.com]

Comment by imran.za...@gmail.com:

Score: Positive

General Comment:
Alex will writing more general sanity check..