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

[Rich Felker]

On Thu, Sep 08, 2005 at 10:36:35PM +0200, Christoph Egger wrote:
> 
> 
> [...]
> > > > 
> > > > 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.
> 
> hmm... see below.
> 
> > > 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)" is wrong, because it compares against the _number_ != 0,
> but arg is a pointer. See below.

False. arg is a pointer type, so if (arg) compares against null
pointer, NOT against numeric zero. Anyway mplayer does not support
systems where the null pointer is not numerically zero (calloc's and
stuff are assumed to initialize null pointers).

> > if (arg != NULL) is just as nonsensical as:
> > if (arg != NULL != 0 != 0 != 0 != 0 != 0 ...)
> 
> Mixing pointers with numbers is often dangerous considering
> sizeof(void *) == sizeof(int) on 32bit platforms, but
> sizeof(void *) != sizeof(int) on 64bit platforms.

This is total nonsense. No mixing of pointers and numbers is taking
place, you just don't know C.

> I already experienced many bugs that caused stack and
> heap corruption just by mixing up pointers with numbers
> - in particular on 64bit platforms.

Again because you don't know C. The code in question is entirely
correct.

Rich