On Thu, Feb 11, 2010 at 10:38:54AM +0000, Daniel Silverstone wrote:
> On Wed, Feb 10, 2010 at 11:37:07PM -0000, [email protected] 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/

Reply via email to