[vorbis-dev] ogg123 remote interface

[Joost Remijn]

On Sun, 6 Oct 2002, Segher Boessenkool wrote:

> Just some comments on getting-rid-of-warnings:
>
> > remove-globals.patch
> >   remove the many global variables from ogg123.c
>
> bad idea for ogg123, imho

Why would it be a bad idea to remove some globals. One of them even had a
comment wich said it should be removed after RC3.

> > make-character-pointers-const.patch
>
> useless.  makes no difference at all for generated code.  const in C does
> not mean the same as const in C++.  it just clobbers up the code.

Here is the explanation from the gcc info page :

`-Wwrite-strings'
     Give string constants the type `const char[LENGTH]' so that
     copying the address of one into a non-`const' `char *' pointer
     will get a warning.  These warnings will help you find at compile
     time code that can try to write into a string constant, but only
     if you have been very careful about using `const' in declarations
     and prototypes.  Otherwise, it will just be a nuisance; this is
     why we did not make `-Wall' request these warnings.

So there is probably no difference in the generated binary, but the code
is cleaner and easier to debug if someone tries to change these strings.

> > remove-some-compiler-warnings.patch
> >   Fixes for some compiler warnings. Unused variable buf commented out.
>
> remove or don't remove, but don't comment out ;)

I commented it out because it looked as if that function could be filled
in later. Should be removed then.

> >   char * initialized to 0 instead of "".
>
> that means something different -- i hope you checked it doesn't
> actually matter

yep i checked. Does it ever make sense to assign "" to a pointer?

> > remove-shadowed-variables.patch
> >   Fix some places in the code where variables are "shadow"ed. These
> >   show up as compiler warnings when compiling with -Wshadow.
>
> bad idea, imnsho.

One example of this was a local variable called optind. It can be
confusing sometimes if people see optind being used and assume it's the
global one. That's why i think it was a good idea.

> > buffer-code-cleanup.patch
> >   Remove unused function free_action. Remove duplicate comment.
>
> fine
>
> > make-playlist-array-null-terminated.patch
> >   Make the playlist NULL terminated. This prevents us from carrying around
> >   the number of items in the playlist later on.
>
> depends -- what about shuffling?  a single int isn't expensive, reading a list
> _is_.

It made the code cleaner. It adds one complete list-walk when shuffling
indeed. The reason i did it was that it became possible to pass the
playlist to a function without also passing the int. Same for returning
the playlist from playlist_create().

> Have fun,
>
> Segher
> --- >8 ----
> List archives:  http://www.xiph.org/archives/
> Ogg project homepage: http://www.xiph.org/ogg/
> To unsubscribe from this list, send a message to 'vorbis-dev-request at xiph.org'
> containing only the word 'unsubscribe' in the body.  No subject is needed.
> Unsubscribe messages sent to the list will be ignored/filtered.
>

--- >8 ----
List archives:  http://www.xiph.org/archives/
Ogg project homepage: http://www.xiph.org/ogg/
To unsubscribe from this list, send a message to 'vorbis-dev-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.