[Dillo-dev]dillo on sparc64
Hi, i have problems with dillo on sparc64. When using a mechanism of OpenBSD 's malloc i'm able to detect a bug in the dpi stuff. At the moment i've found that the function Dpi_append_io_buf() in the file src/IO/dpi.c is wrong. The following line is reallocating a buffer: conn->Buf = g_realloc(conn->Buf, (gulong)io->Status); But when debugging the values of io->Status don't seem to be adequate. I haven't been able to completely fix it as the code is seriously lacking of comment. To be honest i don't even completely understand what this chunk is doing and the code is so unreadable that it doesn't help (not to be harsh but just to give my humble opinion). Damien
Damien COUDERC wrote:
Hi, i have problems with dillo on sparc64. When using a mechanism of OpenBSD 's malloc i'm able to detect a bug in the dpi stuff. At the moment i've found that the function Dpi_append_io_buf() in the file src/IO/dpi.c is wrong.
The following line is reallocating a buffer: conn->Buf = g_realloc(conn->Buf, (gulong)io->Status);
But when debugging the values of io->Status don't seem to be adequate. I haven't been able to completely fix it as the code is seriously lacking of comment. To be honest i don't even completely understand what this chunk is doing and the code is so unreadable that it doesn't help (not to be harsh but just to give my humble opinion).
in the file src/IO/IO.h, in the function static gboolean IO_read(IOData_t *io) there is at line 464 (in last stable release 0.8.4) io->Status = St; but that is true only if not have been read some more data with DataPending = TRUE; which repeate while and it is in the last else that was wrongly commented as /* BytesRead == io->BufSize */ while that conditions is /* BytesRead == io->BufSize */ So in few words the patch I propose is: io->Status = 0; //HERE do { ret = FALSE; DataPending = FALSE; St = read(io->FD, io->Buf, io->BufSize); io->Status += St; //AND HERE obviuosly who read have to free the previous buffer and set io->Status to zero... but in IMHO because the io struct has a flag wich tell us if there is something allocated is a good idea to use it and also free the buffers whose are sent to be read but not freed before. Cheers, -- Roberto A. Foglietta Analista Programmatore GNU/Linux SAD Trasporto Locale S.p.a. Corso Italia 13/N 39100 BOLZANO (I) Tel. +39/0471-450.261 Fax +39/0471-450.253
On Thu, May 19, 2005 at 04:40:55PM +0200, Damien COUDERC wrote:
Hi, i have problems with dillo on sparc64. When using a mechanism of OpenBSD 's malloc i'm able to detect a bug in the dpi stuff. At the moment i've found that the function Dpi_append_io_buf() in the file src/IO/dpi.c is wrong.
The following line is reallocating a buffer: conn->Buf = g_realloc(conn->Buf, (gulong)io->Status);
But when debugging the values of io->Status don't seem to be adequate. I haven't been able to completely fix it as the code is seriously lacking of comment.
All the dpi stuff is under a big revision. The current diff I'm working on is 40 pages. BTW, in the new version, that line is: conn->Buf = g_realloc(conn->Buf, conn->BufSize + (gulong)io->Status);
To be honest i don't even completely understand what this chunk is doing and the code is so unreadable that it doesn't help (not to be harsh but just to give my humble opinion).
:-O -- Jorge.-
Hi Jorge !
The following line is reallocating a buffer: conn->Buf = g_realloc(conn->Buf, (gulong)io->Status);
But when debugging the values of io->Status don't seem to be adequate. I haven't been able to completely fix it as the code is seriously lacking of comment.
All the dpi stuff is under a big revision. The current diff I'm working on is 40 pages. BTW, in the new version, that line is:
conn->Buf = g_realloc(conn->Buf, conn->BufSize + (gulong)io->Status);
That is what i had first done to fix the problem. But dillo still segfault a bit later and i've not been able to found where yet. I can just say that there is a problem with the start of a string that is lost somewhere. IMHO the fix of Roberto looks better but as i'm not used to this code i'm maybe wrong.
To be honest i don't even completely understand what this chunk is doing and the code is so unreadable that it doesn't help (not to be harsh but just to give my humble opinion).
:-O
This is meant to be a constructive critic. I'm addicted to maintainable code and by experience i know that if there are not enough comments the code is not easily readable. For the record, this bug has been found month ago but i've been unmotivated each time i had to read again the dpi.c file. If it had been well commented i should have been able to fix the problem by myself and to send you the resulting patch. Think that you have maybe lost potential developers just for this reason. Now and again this is just my humble opinion and i'm not pretensious enough to try to learn you how to manage your own project. Cheers, Damien
Hi, i still have problems with latest release of dillo on sparc64. I have no more time to waste on this problem so i'm stopping my porting effort for OpenBSD. If you want to fix the problem just look at the following: $ dillo & [1] 5810 $ Nav_open_url: Url=>about:splash< Nav_open_url: Url=>dpi:/bm/< dpi_socket_dir.c:62: tst_dir: access : No such file or directory - /tmp/mips-uonqpe/ debug_msg - init_sockdir: The socket directory /tmp/mips-uonqpe does not exist or is not a directory dpid started Capi_dpi_conn_timeout:: try 1 main.c:154: get_command: dpid tag is NULL : main.c:328: main: get_command failed : bookmarks.dpi (v.13): accepting connections... [fopen]: No such file or directory Dpi_parse_token: [<ÐÐÐÐÐÐÐÐÐÐÐÐÐÐÐsend_page' url='dpi:/bm/'>] [sock_handler_write]: Broken pipe [1] + Segmentation fault dillo (core dumped) Good luck for your project, Damien
participants (3)
-
Damien COUDERC -
Jorge Arellano Cid -
Roberto A. Foglietta