On 08/30/2016 01:21 PM, Pavel Březina wrote:
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 see, good point. Thank you.
I chose go to done. It prevents our users to have wrong configuration.
>> 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.
--
Petr^4 Čech