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

Christoph Egger Christoph_Egger at gmx.de
Thu Sep 8 22:36:35 CEST 2005



[...]
> > > 
> > > 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.

> 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.

The compiler also handles pointers and integers differently.
(Ever wrote a (simple) C compiler?)

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

ANSI C99 defines two integer types always big enough to hold
object pointers: intptr_t and uintptr_t (see POSIX 7.18.1.4)
Mixing anything else with pointers, I call a bug.

Unfortunately, the i386 architecture is pretty forgivable with
these things - sparc64 is the beast on the opposite extreme.

Example: an unaligned memory access has no effect on i386
on any OS. But on sparc64, the CPU throws an exception.
Solaris handles this for the application. NetBSD throws a SIGSEGV
to the application...


> > 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.

My understanding of what is a bug is more restrictive:

Things that possibly lead coders to make mistakes are also a bug.

This includes code, that may be read wrong, tend to make copy/paste
bugs too easy, etc..

Remember: NOBODY is perfect.

-- 
Greetings,

Christoph

Lust, ein paar Euro nebenbei zu verdienen? Ohne Kosten, ohne Risiko!
Satte Provisionen für GMX Partner: http://www.gmx.net/de/go/partner




More information about the MPlayer-dev-eng mailing list