Re: r9965 vince - in /trunk/netsurf/desktop: browser.c browser.h

2010-02-11 Thread Daniel Silverstone
On Wed, Feb 10, 2010 at 11:37:07PM -, nets...@semichrome.net wrote:
 + bw-status_text = NULL;
 + bw-status_text_len = 0;

I see no initialisation of status_match or status_miss

  void browser_window_set_status(struct browser_window *bw, const char *text)

 + if ((bw-status_text == NULL) || (bw-status_text_len  text_len)) {
 + /* no current string allocation or it is not long enough */
 + free(bw-status_text);

While free(NULL) is defined to be safe -- are we certain it is on all platforms
we port to?

 Modified: trunk/netsurf/desktop/browser.h
 +
 +/** cache of the currently displayed status text. */
 + char *status_text;
 + int status_text_len;
 + int status_match;
 + int status_miss;

The incorrect indentation indicates that the comment was space-indented rather 
than tabbed, and the three ints lack documentation strings.

How about:

char *status_text; /** Current status bar text. */
int status_text_len; /** Length of the ::status_text buffer. */
int status_match; /** Number of times an idempotent status-set 
operation was performed. */
int status_miss; /** Number of times status was really updated. */

?

-- 
Daniel Silverstone   http://www.netsurf-browser.org/
PGP mail accepted and encouraged.Key Id: 3CCE BABE 206C 3B69



Re: r9965 vince - in /trunk/netsurf/desktop: browser.c browser.h

2010-02-11 Thread Vincent Sanders
On Thu, Feb 11, 2010 at 10:38:54AM +, Daniel Silverstone wrote:
 On Wed, Feb 10, 2010 at 11:37:07PM -, nets...@semichrome.net wrote:
  +   bw-status_text = NULL;
  +   bw-status_text_len = 0;
 
 I see no initialisation of status_match or status_miss
 
   void browser_window_set_status(struct browser_window *bw, const char *text)
 
  +   if ((bw-status_text == NULL) || (bw-status_text_len  text_len)) {
  +   /* no current string allocation or it is not long enough */
  +   free(bw-status_text);
 
 While free(NULL) is defined to be safe -- are we certain it is on all 
 platforms
 we port to?

A good question...I kinda assumed the C default behaviour...ummm if
anyone has an issue with this in a build can they let me know? if it
is an issue I will of course fix it.

 
  Modified: trunk/netsurf/desktop/browser.h
  +
  +/** cache of the currently displayed status text. */
  +   char *status_text;
  +   int status_text_len;
  +   int status_match;
  +   int status_miss;
 
 The incorrect indentation indicates that the comment was space-indented 
 rather than tabbed, and the three ints lack documentation strings.
 
 How about:
 
   char *status_text; /** Current status bar text. */
   int status_text_len; /** Length of the ::status_text buffer. */
   int status_match; /** Number of times an idempotent status-set 
 operation was performed. */
   int status_miss; /** Number of times status was really updated. */
 
 ?
 

all applied and the indentation/whitespace damage of the whole file
fixed, cheers Daniel

-- 
Regards Vincent
http://www.kyllikki.org/



Re: r9965 vince - in /trunk/netsurf/desktop: browser.c browser.h

2010-02-11 Thread Bernd Roesch
Hello 

On 11.02.10, you wrote:

 
 A good question...I kinda assumed the C default behaviour...ummm if
 anyone has an issue with this in a build can they let me know? if it
 is an issue I will of course fix it.

free can use with 0 Pointer and should do nothing as the ISO 9899 say

http://www.open-std.org/JTC1/SC22/wg14/www/docs/n1124.pdf

there is only a diffrence possible in malloc.when call malloc with size 0
its possible to return 0 or a pointer.


Regards




Re: r9965 vince - in /trunk/netsurf/desktop: browser.c browser.h

2010-02-11 Thread Rob Kendrick
On Thu, 11 Feb 2010 15:44:28 +0200
Bernd Roesch nospamn...@gmx.de wrote:

 Hello 
 
 On 11.02.10, you wrote:
 
  
  A good question...I kinda assumed the C default behaviour...ummm if
  anyone has an issue with this in a build can they let me know? if it
  is an issue I will of course fix it.
 
 free can use with 0 Pointer and should do nothing as the ISO 9899 say

Of course.  But the issue is Do we care about running anywhere that
ignores the standard in this respect?

It's not unheard of for free() to explode when being passed NULL.

B.



Re: r9965 vince - in /trunk/netsurf/desktop: browser.c browser.h

2010-02-11 Thread James Bursa
On Thursday 11 February 2010, Daniel Silverstone wrote:

 While free(NULL) is defined to be safe -- are we certain it is on all
  platforms we port to?

I think relying on this is common throughout the code. I know a lot of the 
code I wrote on assumes it's ok, so a port to a platform without it would 
probably crash quickly.

James

-- 
James Bursa, NetSurf developerhttp://www.netsurf-browser.org/