On 12/04/2015 04:42 PM, Jakub Hrozek wrote:
On Thu, Dec 03, 2015 at 01:24:54PM +0100, Pavel Březina wrote:
> On 11/20/2015 12:04 PM, Jakub Hrozek wrote:
>> On Thu, Nov 19, 2015 at 01:51:54PM +0100, Pavel Březina wrote:
>>>> Thanks, new patches are attached.
>>>
>>> I had a phone call with Jakub and we decided that it will be better to use
>>> be_req directly instead of lower level sbus_req. This will allow us to
>>> simplify it more.
>>
>> OK, see the attached patches..
>
> Hi, thanks for the changes.
>
> First patch:
>
>> @@ -199,6 +205,7 @@ struct be_req *be_req_create(TALLOC_CTX *mem_ctx,
>> be_req->domain = be_ctx->domain;
>> be_req->fn = fn;
>> be_req->pvt = pvt_fn_data;
>> + be_req->req_name = name;
>
> I know every call now passes static data but IMHO we should be safe and use
> talloc_strdup here.
>
>> +static errno_t be_sbus_reply(struct sbus_request *sbus_req,
>> + dbus_uint16_t err_maj,
>> + dbus_uint32_t err_min,
>> + const char *err_msg)
>> +{
>> + errno_t ret;
>> + const char *safe_err_msg;
>> +
>> + /* Only return a reply if one was requested
>> + * There may not be one if this request began
>> + * while we were offline
>> + */
>> + if (sbus_req == NULL) {
>> + return EOK;
>> + }
>> +
>> + safe_err_msg = safe_be_req_err_msg(err_msg, err_maj);
>> +
>> + if (err_maj == DP_ERR_FATAL && err_min == ENODEV) {
>> + DEBUG(SSSDBG_TRACE_LIBS, "Handler not configured\n");
>> + } else {
>> + DEBUG(SSSDBG_TRACE_LIBS,
>> + "Request processed. Returned %d,%d,%s\n",
>> + err_maj, err_min, err_msg);
>> + }
>
> Can we translate err_maj into string here? To get message similar to:
> "Request processed: Success [%d]: %s, err_min, err_msg
> "Request processed: Failure [%d]: %s, err_min, err_msg
> "Request processed: Offline [%d]: %s, err_min, err_msg
>
> Second patch:
>
>> +#define BE_SBUS_REPLY_DATA_INIT { .err_maj = DP_ERR_FATAL, \
>> + .err_min = EFAULT, \
>> + .err_msg = "Fatal error" \
>> + };
>
> ERR_INTERNAL might be a better choice over EFAULT since it basically means
> we forgot to set it -> code error.
OK, see the attached two.
Ack.