On 08/30/2016 01:06 PM, Lukas Slebodnik wrote:
On (30/08/16 13:03), Petr Cech wrote:
> On 08/30/2016 12:42 PM, Pavel Březina wrote:
>> On 08/25/2016 01:43 PM, Petr Cech wrote:
>>> - /* FIXME: get max_children from configuration file */
>>> - auth_ctx->max_children = 10;
>>> + ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
>>> + CONFDB_PROXY_MAX_CHILDREN, 10,
>>> + &max_children);
>>> + if (ret != EOK) {
>>> + DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]:
%s\n",
>>> + ret, sss_strerror(ret));
>>> + goto done;
>>> + }
>>> + if (max_children < 1) {
>>> + DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then
1\n",
>>> + CONFDB_PROXY_MAX_CHILDREN);
>>> + goto done;
>>> + }
>>
>> You need to either set ret here, or set max_children to some reasonable
>> value (10?).
>
> Hello Pavel,
>
> max_children is set on the next line as:
> auth_ctx->max_children = max_children;
No it is not.
+ if (ret != EOK) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+ ret, sss_strerror(ret));
+ goto done;
+ }
+ if (max_children < 1) {
+ DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+ CONFDB_PROXY_MAX_CHILDREN);
+ goto done;
+ }
+ auth_ctx->max_children = max_children;
if max_children < 1, you report an error and goto done, keeping ret =
EOK from previous successful call. Either we consider it as an error,
and set ret to e.g. EINVAL, or we set default value and continue.
>
> I use temporary variable max_children,
> because there is issue with signed/unsigned
> integer value.
>
> 10 was original value. I increase it to 50.
> And I use constant OPT_MAX_CHILDREN_DEFAULT now.
>
10 is a reasonable default and works for most of users.
I do not think we need to increase default value.
This is a purpose of the new option
Agree.