RE: [AD] Five patches

["Vincent Penquerc'h" <Vincent.Penquerch@xxxxxxxxxx>]

Title: RE: [AD] Five patches

Annie's comments, sorry for the delay.

--
Vincent Penquerc'h

** UBUFFERS

After sending the patch, I've thought they may be better as an add-on.  So,
it's all right if they don't go in the lib.  But I can't completly agree with
your remarks.

Eric Botcazou wrote:
> current mechanism based on uconvert() is relatively light and smart enough
> to do nothing if no conversion is needed.

IMO, uconvert makes the code less readable, particularly with a bunch of
char tmp[] at the beginning of a function (not speaking of the hard-coded
buffer length).  Another problem is that the string can be truncated if the
buffer is too short.  It's true that uconvert is quicker than UBUFFERS, so
there are cases when you have to choose uconvert, but I don't think it's often.
(In a program I wrote -a map editor- there are more than 250 conversions in the
code, but speed is a concern only in three cases).  Besides, UBUFFERS can be
"smarter": it is possible to convert a string only when needed, it is possible
to use a circular buffer to limit the memory used, and more... without changing
the API.  But, again, this should go in an add-on.

In fact, I'm not really happy with UBUFFERS, for the same reason I'm not
happy with uconvert: using them is too complex.  It's OK when you
have tight speed or memory limitations to trade simplicity for optimization,
but for the general case I'd prefer something easy to use and error proof.

** pack_fdopen

Eric Botcazou wrote:
> Documenting the limitation in the docs is sufficient I think.

"Speed is not a concern, therefore catch errors." (c) Vincent Penquerc'h and
"Catch errors early." (c) Annie Testes ;)
If the mode errors are not caught, i think the program will crash with
the next read or write, and the programmer may have hard time fixing the
problem.  And for a partially written/read file, the program behavior may
seem random, making the fix even harder.
Besides, I wanted pack_fdopen to mimic fdopen behavior.

Eric Botcazou wrote:
> ... More generally, underscored functions are
> reserved for non static internal functions.

Thanks for the info.  Sorry for breaking compilation.

** XWindow hang

Peter Wang wrote:
> >   * Under XWindow, this code hangs:
> >         #include <allegro.h>
> >         int main(void)
> >         {
> >            allegro_init();
> >            return 0;
> >         }
> >         END_OF_MAIN()
> >   Seems to hang inside _xwin_destroy_window and seems to be related to pthread
> >   The ps command shows that there are 2 processes.
>
> I can't reproduce it.  It could be a race condition.

In the function _xwin_sysdrv_exit, _unix_bg_man->exit() is called *before*
_xwin_destroy_window and _xwin_close_display, which both use _unix_bg_man
inside the macros XLOCK and XUNLOCK.  Moving the call to _unix_bg_man->exit()
after these 2 functions seems to fix the problem. I.e. :
        static void _xwin_sysdrv_exit(void)
        {
          _unix_bg_man->exit();
          _xwin_destroy_window();
          _xwin_close_display();
becomes:
        static void _xwin_sysdrv_exit(void)
        {
          _xwin_destroy_window();
          _xwin_close_display();
          _unix_bg_man->exit();
Since I don't know threads and XWindow, I'm not sure if this is really a fix
or just random luck.

** fbcon patch

> > *** Another patch, for fbcon: convert the strings when needed...
>
> Committed.

I've just discovered I had let some shameless comments in the patch I sent.
Sorry.

--
Annie Testes