Remotelogging stuff (Re: Revision 4f37cd..)

[Shkutkov Michael]

Sadrul Habib Chowdhury wrote:
> * Shkutkov Michael had this to say on [24 Jul 2007, 16:15:46 +0400]:
>   
>> Sadrul Habib Chowdhury wrote:
>>     
>>> * Does it make sense to send the account and name to the callbacks
>>> (PurpleLogListCallback etc.)?
>>>       
>> Actually I don't this so, this is an extra data which isn't really 
>> necessary. Look at gtklog.c
>> (remotelogging branch) you could see that account and name in 
>> PurpleLogListCallback aren’t used.
>> And I think it's really easy to pass them in callback through user data if 
>> necessary.
>>     
>
> Sending relevant information to callbacks is useful, even if it's not
> guaranteed to be used. We do so for various signal callbacks, I suggest
> we do the same for the logging callbacks too.
>
>   
Actually, I also think that it maybe useful, but which information is 
relevant: all arguments of nonblocking
log function? And in which way we should send this inforation to the 
callback?
I guess we can save current nonblocking log function signature and use 
last argument for this purpose. So it
could be

//PurpleLogListCallback
void purple_log_list_cb(GList *list, void *data) {
    PurpleLogListCallbackData *cb_data = data;
    SomeUserData user_data = cb_data->data;
    PurpleLogType type = cb_data->type;
    const char *name  = cb_data->name;
    PurpleAccount *account = cb_data->account;
    ...  
}

>>> * Instead of maintaining a counter in the callback data, does it make
>>> sense to use something like a PurpleLogContext or something)? For example,
>>>
>>>   PurpleLogContext *ct = purple_log_context_new(destroy_context_cb);
>>>   purple_log_context_set_userdata(ct, some_data);
>>>   purple_log_read_nonblocking(ct, ..., callback_fn, ...);
>>>
>>> and the _nonblocking functions will _ref the 'ct', and an internal
>>> log-callback (not 'callback_fn') will _unref it, and call
>>> 'destroy_context_cb' when appropriate.
>>>
>>> Am I making any sense, or making things unnecessarily more complex than
>>> they need to be?
>>>       
>> First of all we need counter if we have several nonblocking log calls like 
>> in gntblist.c (because we
>>     
>
> The non-blocking logging stuff in gntblist.c was rewritten, and it
> doesn't use counter any more (17ea40ad6c737481f69dd3a4fb680156a13b6f44
> and 20903902f45841cb1736b39abeb131b80a7a7d86).
>
>   
Thanks. Now I've got some new ideas.
>> need to know when all calls will finish, to make some things and free user 
>> data)
>> If we have only one nonblocking log call we don't need counter and context 
>> too
>> (So it seems that rather often PurpleLogContext is unnecessary)
>>
>> I guess you would like to simplify all staff related to counter.
>> I've thought about this, but question is when it will be appropriate to 
>> call destroy_context_cb?
>> We can't do this if ref count is zero, because not all loggers implement 
>> their methods in nonblocking style
>> (and this is all right, we need only those that have big delays). So it's 
>> really often when ref is zero,
>> but we haven’t finished so we shouldn't call destroy_context_cb. (Maybe 
>> it's hard to understand)
>>     
>
> It is a little hard to understand, especially why we can't call
> destroy_context_cb when ref-count is zero. Can't we determine when an
> operation (blocking or not) has ended? If we can, can't we unref the
> context only then (and call destroy_context_cb when it reaches zero)?
> We now use 'counter', or 'finished' or 'need_continue' etc. to keep
> track of the user-data, and it looks like there needs to be a simpler
> way of accessing logging data from other places (eg., the buddylist,
> log viewer, plugins etc.)
>
>   
>> Another problem, is how free user data (which stored in context). We should 
>> do this after last
>> nonblocking call, but how to determine it?
>>     
>
> When the ref-count reaches zero (ie., from 'destroy_context_cb'). In the
> above code snippet, whoever is creating 'ct' is the owner of
> 'some_data', and should thus be able to do necessary destroy operations
> from 'destroy_context_cb'.
>
>   
Sorry, I've missed something. So everything will be all right with log 
context.
On the one hand context simplify life a litte on other hand we should 
have several callbacks and
extra argument for each log function.
(I guess we should use last log functions argument for context, which 
now used for user data,

  PurpleLogContext *ct = purple_log_context_new(destroy_context_cb);
  purple_log_context_set_userdata(ct, some_data);
  purple_log_read_nonblocking(..., callback_fn, ct);

)


-- 
With best regards,
Shkutkov Michael