- Dillo-dev - lists.mailman3.com

Warnings in src/IO/dpi.c and lout/misc.hh

by newman.x＠gmail.com

On Wed, Apr 01, 2009 at 10:16:41PM +0200, Hofmann Johannes wrote: > On Fri, Mar 27, 2009 at 01:07:14PM +0100, Hofmann Johannes wrote: > > Hi, > > > > On Fri, Mar 27, 2009 at 12:00:46AM +0100, Michal Nowak wrote: > > > Found this three warnings when compiling with gcc-4.3.2-7.i386: > > > > > > gcc -DHAVE_CONFIG_H -I. -I../.. -I/usr/local/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -g -O2 -DD_DNS_THREADED -D_REENTRANT -D_THREAD_SAFE -Wall -W -Waggregate-return -MT dpi.o -MD -MP -MF .deps/dpi.Tpo -c -o dpi.o dpi.c > > > dpi.c: In function ?Dpi_get_dpid_uds_dir?: > > > dpi.c:382: warning: ignoring return value of ?fgets?, declared with attribute warn_unused_result > > > dpi.c: In function ?a_Dpi_bye_dpid?: > > > dpi.c:733: warning: ignoring return value of ?write?, declared with attribute warn_unused_result > > > > > > g++ -DHAVE_CONFIG_H -I. -I.. -DDILLORC_SYS='"/usr/local/etc/dillorc"' -I/usr/local/include -I.. -I/usr/include/libpng12 -I/usr/include/freetype2 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -Wno-non-virtual-dtor -g -O2 -Wall -W -Wno-unused-parameter -fno-rtti -fno-exceptions -MT styleengine.o -MD -MP -MF .deps/styleengine.Tpo -c -o styleengine.o styleengine.cc > > > ../lout/misc.hh: In member function ?void StyleEngine::apply(dw::core::style::StyleAttrs*, CssPropertyList*)?: > > > ../lout/misc.hh:181: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false > > > > Can you please check whether attached patches fix the warnings? > > Michal, do the patches fix the warnings for you? > > Cheers, > Johannes Yes, the mentioned warnings are fixed now, thanks Johannes! But one more emerged due to recent clean up, see follow-up email. Michal > > > > > @all: please have a look at the dpi.c fix. > > > > Cheers, > > Johannes > > > diff -r b06377790b00 src/IO/dpi.c > > --- a/src/IO/dpi.c Thu Mar 26 23:18:02 2009 +0000 > > +++ b/src/IO/dpi.c Fri Mar 27 13:05:11 2009 +0100 > > @@ -371,7 +371,7 @@ static char *Dpi_get_dpid_uds_dir(void) > > { > > FILE *in; > > char *saved_name_filename; /* :) */ > > - char dpid_uds_dir[256], *p = NULL; > > + char buf[256], *dpid_uds_dir, *p = NULL, *p1; > > > > saved_name_filename = > > dStrconcat(dGethomedir(), "/.dillo/dpi_socket_dir", NULL); > > @@ -379,14 +379,16 @@ static char *Dpi_get_dpid_uds_dir(void) > > dFree(saved_name_filename); > > > > if (in != NULL) { > > - fgets(dpid_uds_dir, 256, in); > > + dpid_uds_dir = fgets(buf, sizeof(buf), in); > > fclose(in); > > - if ((p = strchr(dpid_uds_dir, '\n'))) { > > - *p = 0; > > - } > > - if (access(dpid_uds_dir, F_OK) == 0) { > > - p = dStrdup(dpid_uds_dir); > > - _MSG("Dpi_get_dpid_uds_dir:: %s\n", p); > > + if (dpid_uds_dir) { > > + if ((p1 = strchr(dpid_uds_dir, '\n'))) { > > + *p1 = 0; > > + } > > + if (access(dpid_uds_dir, F_OK) == 0) { > > + p = dStrdup(dpid_uds_dir); > > + _MSG("Dpi_get_dpid_uds_dir:: %s\n", p); > > + } > > } > > } > > > > @@ -730,7 +732,9 @@ void a_Dpi_bye_dpid() > > MSG("%s\n", sa.sun_path); > > } > > DpiBye_cmd = a_Dpip_build_cmd("cmd=%s", "DpiBye"); > > - (void) write(new_socket, DpiBye_cmd, strlen(DpiBye_cmd)); > > + if (write(new_socket, DpiBye_cmd, strlen(DpiBye_cmd)) != > > + (ssize_t) strlen(DpiBye_cmd)) > > + MSG_WARN("Failed to send DpiBye\n"); > > dFree(DpiBye_cmd); > > Dpi_close_fd(new_socket); > > } > > > diff -r b06377790b00 lout/misc.hh > > --- a/lout/misc.hh Thu Mar 26 23:18:02 2009 +0000 > > +++ b/lout/misc.hh Fri Mar 27 12:52:36 2009 +0100 > > @@ -178,7 +178,7 @@ public: > > */ > > inline T get (int i) { > > if (BOUND_CHECKING) > > - assert (i >= 0 && i < this->num); > > + assert (i >= 0 && this->num - i > 0); > > return this->array[i]; > > } > > > > @@ -192,7 +192,7 @@ public: > > */ > > inline void set (int i, T t) { > > if (BOUND_CHECKING) > > - assert (i >= 0 && i < this->num); > > + assert (i >= 0 && this->num - i > 0); > > this->array[i] = t; > > } > > }; > > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev -- Regards, Michal Nowak

April 1, 2009

Warnings in src/IO/dpi.c and lout/misc.hh

by Johannes.Hofmann＠gmx.de

On Fri, Mar 27, 2009 at 01:07:14PM +0100, Hofmann Johannes wrote: > Hi, > > On Fri, Mar 27, 2009 at 12:00:46AM +0100, Michal Nowak wrote: > > Found this three warnings when compiling with gcc-4.3.2-7.i386: > > > > gcc -DHAVE_CONFIG_H -I. -I../.. -I/usr/local/include -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -g -O2 -DD_DNS_THREADED -D_REENTRANT -D_THREAD_SAFE -Wall -W -Waggregate-return -MT dpi.o -MD -MP -MF .deps/dpi.Tpo -c -o dpi.o dpi.c > > dpi.c: In function ?Dpi_get_dpid_uds_dir?: > > dpi.c:382: warning: ignoring return value of ?fgets?, declared with attribute warn_unused_result > > dpi.c: In function ?a_Dpi_bye_dpid?: > > dpi.c:733: warning: ignoring return value of ?write?, declared with attribute warn_unused_result > > > > g++ -DHAVE_CONFIG_H -I. -I.. -DDILLORC_SYS='"/usr/local/etc/dillorc"' -I/usr/local/include -I.. -I/usr/include/libpng12 -I/usr/include/freetype2 -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i386 -mtune=generic -fasynchronous-unwind-tables -Wno-non-virtual-dtor -g -O2 -Wall -W -Wno-unused-parameter -fno-rtti -fno-exceptions -MT styleengine.o -MD -MP -MF .deps/styleengine.Tpo -c -o styleengine.o styleengine.cc > > ../lout/misc.hh: In member function ?void StyleEngine::apply(dw::core::style::StyleAttrs*, CssPropertyList*)?: > > ../lout/misc.hh:181: warning: assuming signed overflow does not occur when assuming that (X - c) > X is always false > > Can you please check whether attached patches fix the warnings? Michal, do the patches fix the warnings for you? Cheers, Johannes > > @all: please have a look at the dpi.c fix. > > Cheers, > Johannes > diff -r b06377790b00 src/IO/dpi.c > --- a/src/IO/dpi.c Thu Mar 26 23:18:02 2009 +0000 > +++ b/src/IO/dpi.c Fri Mar 27 13:05:11 2009 +0100 > @@ -371,7 +371,7 @@ static char *Dpi_get_dpid_uds_dir(void) > { > FILE *in; > char *saved_name_filename; /* :) */ > - char dpid_uds_dir[256], *p = NULL; > + char buf[256], *dpid_uds_dir, *p = NULL, *p1; > > saved_name_filename = > dStrconcat(dGethomedir(), "/.dillo/dpi_socket_dir", NULL); > @@ -379,14 +379,16 @@ static char *Dpi_get_dpid_uds_dir(void) > dFree(saved_name_filename); > > if (in != NULL) { > - fgets(dpid_uds_dir, 256, in); > + dpid_uds_dir = fgets(buf, sizeof(buf), in); > fclose(in); > - if ((p = strchr(dpid_uds_dir, '\n'))) { > - *p = 0; > - } > - if (access(dpid_uds_dir, F_OK) == 0) { > - p = dStrdup(dpid_uds_dir); > - _MSG("Dpi_get_dpid_uds_dir:: %s\n", p); > + if (dpid_uds_dir) { > + if ((p1 = strchr(dpid_uds_dir, '\n'))) { > + *p1 = 0; > + } > + if (access(dpid_uds_dir, F_OK) == 0) { > + p = dStrdup(dpid_uds_dir); > + _MSG("Dpi_get_dpid_uds_dir:: %s\n", p); > + } > } > } > > @@ -730,7 +732,9 @@ void a_Dpi_bye_dpid() > MSG("%s\n", sa.sun_path); > } > DpiBye_cmd = a_Dpip_build_cmd("cmd=%s", "DpiBye"); > - (void) write(new_socket, DpiBye_cmd, strlen(DpiBye_cmd)); > + if (write(new_socket, DpiBye_cmd, strlen(DpiBye_cmd)) != > + (ssize_t) strlen(DpiBye_cmd)) > + MSG_WARN("Failed to send DpiBye\n"); > dFree(DpiBye_cmd); > Dpi_close_fd(new_socket); > } > diff -r b06377790b00 lout/misc.hh > --- a/lout/misc.hh Thu Mar 26 23:18:02 2009 +0000 > +++ b/lout/misc.hh Fri Mar 27 12:52:36 2009 +0100 > @@ -178,7 +178,7 @@ public: > */ > inline T get (int i) { > if (BOUND_CHECKING) > - assert (i >= 0 && i < this->num); > + assert (i >= 0 && this->num - i > 0); > return this->array[i]; > } > > @@ -192,7 +192,7 @@ public: > */ > inline void set (int i, T t) { > if (BOUND_CHECKING) > - assert (i >= 0 && i < this->num); > + assert (i >= 0 && this->num - i > 0); > this->array[i] = t; > } > };

April 1, 2009

Segfault on sites requiring authentication

by corvid＠lavabit.com

Tim wrote: > > I'm not seeing this crash. Is it going through the for loop and > > getting set to NULL in Client_bw = ClientWeb->bw;, or is it > > NULL because there weren't any clients and it didn't execute that code? > > Yes, it is going through the loop. Probably comparing Client->Url > with entry->Url fails. As a result, ClientWeb and Client_bw do not > get set. It would be good to find out what it's doing exactly for you. - I'd expect a client to be waiting for it in this case. - I think a client shouldn't ever have a NULL bw. - The case where there are no clients waiting for the URL does sound like a bug, although I think I'd be tempted just not to call Cache_auth_entry() if the bw was NULL. > Have you also tried reloading the page? Sometimes it does not get > loaded properly here. Anyway, it is odd that Dillo does not crash > for you. Are you using the latest trunk? Reloading doesn't crash it for me. Yes, I'm using the latest code. You mentioned the other day that you get a lot of crashes in general, and I was surprised because I've had a total of three random crashes since 2.0 was released.

April 1, 2009

[PATCH] startup window

by tim.nieradzik＠gmx.de

Heh, that is what I had in mind all the time. However, nice idea for an April Fool's hoax. :) --Tim

April 1, 2009

Segfault on sites requiring authentication

by tim.nieradzik＠gmx.de

> I'm not seeing this crash. Is it going through the for loop and > getting set to NULL in Client_bw = ClientWeb->bw;, or is it > NULL because there weren't any clients and it didn't execute that code? Yes, it is going through the loop. Probably comparing Client->Url with entry->Url fails. As a result, ClientWeb and Client_bw do not get set. Have you also tried reloading the page? Sometimes it does not get loaded properly here. Anyway, it is odd that Dillo does not crash for you. Are you using the latest trunk? --Tim

April 1, 2009

[PATCH] startup window

by Johannes.Hofmann＠gmx.de

On Wed, Apr 01, 2009 at 04:49:44PM +0000, corvid wrote: > Thomas wrote: > > Am Wed, 1 Apr 2009 09:47:11 +0200 > > schrieb Hofmann Johannes <Johannes.Hofmann(a)gmx.de>: > > > > > Hi, > > > attached patch adds a proper startup screen for dillo. > > > > Oh, is dillo slow enough now that it needs a splash screen distracting the user while waiting for startup? > > I sort of like the "instant startup" behaviour. A splash startup screen reminds you of the startup _not_ being instant. > > Especially opening an extra window wastes CPU cycles, even in X11, that could be used on doing the actual work... > > Of course I'll have to check out a current dillo2 to see how eminent the issue really is. But when we reach the startup speed of e.g. the GIMP with it's endless loading of plugins etc., one can safely declare dillo "just another bloated browser". > > I don't want to be offensive, just voice my concern... > > I agree with Thomas here. > me too - check the date :)

April 1, 2009

[PATCH] startup window

by corvid＠lavabit.com

Thomas wrote: > Am Wed, 1 Apr 2009 09:47:11 +0200 > schrieb Hofmann Johannes <Johannes.Hofmann(a)gmx.de>: > > > Hi, > > attached patch adds a proper startup screen for dillo. > > Oh, is dillo slow enough now that it needs a splash screen distracting the user while waiting for startup? > I sort of like the "instant startup" behaviour. A splash startup screen reminds you of the startup _not_ being instant. > Especially opening an extra window wastes CPU cycles, even in X11, that could be used on doing the actual work... > Of course I'll have to check out a current dillo2 to see how eminent the issue really is. But when we reach the startup speed of e.g. the GIMP with it's endless loading of plugins etc., one can safely declare dillo "just another bloated browser". > I don't want to be offensive, just voice my concern... I agree with Thomas here.

April 1, 2009

Segfault on sites requiring authentication

by corvid＠lavabit.com

Tim wrote: > Whenever I visit http://puppyisos.org/isos (puppy / linux), Dillo > segfaults. The reason is that in some cases the variable Client_bw > is NULL. However, some functions depend on it and access its member > variables. > > Client_bw is initialized in Cache_process_queue() (src/cache.c). > When authentication is required, this variable is passed over to > Cache_auth_entry() which sets a timeout. Its callback > (Cache_auth_callback()) opens up a dialog asking for the credentials > and then calls a_Nav_reload() with the Client_bw as its first > parameter. Finally in a_Nav_cancel_expect() Client_bw's member > variable nav_expecting is used and as bw is NULL, Dillo segfaults. I'm not seeing this crash. Is it going through the for loop and getting set to NULL in Client_bw = ClientWeb->bw;, or is it NULL because there weren't any clients and it didn't execute that code? > I came up with a small patch that removes Cache_auth_callback() and > Cache_auth_entry() as I have not seen any advantages in delaying the > authentication popup. The next thing I did was to show the dialog on > condition that Client_bw is not NULL and that we have finished the > transfer (At least I hope CA_Close is the proper flag for doing so). > > There were various unnecessary checks in src/nav.c. They compared bw > with NULL. In my opinion the affected functions (a_Nav_expect_done, > a_Nav_push and a_Nav_repush) should never be called if you cannot be > sure that bw is not NUL. What is your view on this subject? > > Unfortunately, my approach does not solve the original problem that > caused Client_bw being NULL. Furthermore, the patch introduces a new > bug: The dialog pops up when reloading the site but after having > entered the correct credentials or having pressed the ``Cancel'' > button, it appears again. Any ideas? > > While digging deeper into the code, I noticed that we make heavy use > of timers for popups and reloading pages? Is there any special > reason for it? If the calling code needs to go back to whatever it was doing rather than waiting around for the user to interact with a popup, a timer is used.

April 1, 2009

[PATCH] startup window

by thomas-forum＠orgis.org

Am Wed, 1 Apr 2009 09:47:11 +0200 schrieb Hofmann Johannes <Johannes.Hofmann(a)gmx.de>: > Hi, > attached patch adds a proper startup screen for dillo. Oh, is dillo slow enough now that it needs a splash screen distracting the user while waiting for startup? I sort of like the "instant startup" behaviour. A splash startup screen reminds you of the startup _not_ being instant. Especially opening an extra window wastes CPU cycles, even in X11, that could be used on doing the actual work... Of course I'll have to check out a current dillo2 to see how eminent the issue really is. But when we reach the startup speed of e.g. the GIMP with it's endless loading of plugins etc., one can safely declare dillo "just another bloated browser". I don't want to be offensive, just voice my concern... Alrighty then, Thomas.

April 1, 2009

[PATCH] startup window

by Johannes.Hofmann＠gmx.de

Hi, attached patch adds a proper startup screen for dillo. Cheers, Johannes

April 1, 2009