[MPlayer-dev-eng] [PATCH] libvo: GGI driver update

[Rich Felker]

On Thu, Sep 08, 2005 at 09:09:32PM +0200, Christoph Egger wrote:
> 
> > Hi,
> > On Thu, Sep 08, 2005 at 08:10:07PM +0200, Christoph Egger wrote:
> > > > How about addressing Reimar's review now?  He had some questions and
> > > > suggested some improvements.
> > > 
> > > I did. I changed the if in get_image() as he suggested.
> > > 
> > > And to answer his question about ggi_conf.async - this variable
> > > was only used for debugging and thus removed. The actual asynchronous
> > > mode is set via ggiSetFlags()
> > 
> > That leaves the questions why you moved some things (draw_slice
> > function, async setting, ), and I consider this cosmetics:
> > -    if ((char *) arg) {
> > +    if (arg != NULL) {
> >        int i = 0;
> >        ggi_conf.driver = strdup(arg);
> > -        while (ggi_conf.driver[i]) {
> > +        while (ggi_conf.driver[i] != '\0') {
> 
> Why do you call this a cosmetic change?

Anything that does not alter functionality is cosmetic change.

> The compilers parser, syntax analyser, etc. handles this differently.
> And the internal syntax tree is also different.
> 
> the 'arg' is a const char * by function parameter.
> Casting it to char * disqualifies this datatype for no reason.

Casting it at all is nonsense. if (arg) is the correct code..
if (arg != NULL) is just as nonsensical as:
if (arg != NULL != 0 != 0 != 0 != 0 != 0 ...)

> I call this a bug. Don't know what you ever learned about
> type safety (in C, pascal, java, etc.).

It's not a bug unless it can possibly lead to wrong behavior.

Rich