lin-club  

Re: r2llib interface change requests

mulix
Fri, 03 Aug 2001 15:49:46 -0700

On Fri, 3 Aug 2001, guy keren wrote:

>
> On Fri, 3 Aug 2001, mulix wrote:
>
> > > 2. there should be a single function that gets both r2l mode and biditext
> > >  base directionality - using a single stat() call. having to call 2
> > >  seperate functions is inefficient - especially inside biditext's string
> > >  drawing code...
> >
> > this becomes largely irrelevant if we switch from file + size to two
> > different files...
> >
> > in that case, i doubt the interface ugliness will be worth it for the
> > little gain of saving one function call.
>
> no, no. this function is in the critical path (teh one used by the
> biditext library, on each string drawing). now think of a desktop with
> several applications, several windows for each, all trying to perform a
> screen redraw....

i guess you didnt get my meaning above.
if we switch from file existence (r2l state) + size (biditext base dir)
to two files (one for r2l state and one for biditext base dir), then we
have to have *two* stat()s, anyway.

if we need to have two stat()s, i dont like the ugly interface.

> and its not a function call - its a system call. much more costly...
> and the interface isn't realy ugly. it just gets an r2l instance, and 2
> poitners - one for an R2L_STATE variable, and one for a
> BIDITEXT_BASE_STATE variable. something like:
>
> int r2l_query_state(r2llib_t rtl,
>                     R2L_STATE* r2l_state,
>                     BIDITEXT_BASE_STATE* bt_state);
>
> this is not so ugly, and more efficient for the critical path...

it is ugly, since it forces me to do one of the following:

* implement it in terms of already existing functions - we win exactly
one function invocation in that case.

int r2l_query_state(r2llib_t rtl,
                     R2L_STATE* r2l_state,
                     BIDITEXT_BASE_STATE* bt_state)
{
        assert(rtl && r2l_state && bt_state);

        *r2l_state = r2l_get_current_r2l_state(rtl);
        *bt_state = r2l_get_current_biditext_base_dir(rtl);
}

* implement it by cutting and pasting (hideous) or reorganizing the
existing code to use this function, forcing a much tighter coupling of
the r2l state and biditext base state.

int r2l_query_state(r2llib_t rtl,
                     R2L_STATE* r2l_state,
                     BIDITEXT_BASE_STATE* bt_state)
{
        assert(rtl)

        if (r2l_state)
                *r2l_state = knowledge specific to r2l_state
        if (bt_state)
                *bt_state = knowledge specific to bt_state
}

R2L_STATE
r2l_get_current_r2l_state(r2llib_t rtl)
{
        assert(rtl);

        R2L_STATE res;
        r2l_query_state(rtl, &res, NULL);

        return res;
}

we win nothing in either case. the only reason to have such an interface
is if it gives us *real wins* over the seperate function for each state.
for the "two files" case, i dont see how it can do that.

-- 
mulix
http://www.advogato.com/person/mulix

linux/reboot.h: #define LINUX_REBOOT_MAGIC1 0xfee1dead