Vorbis dev - Dec 2001 - Nonportable use of strdup()

Hey, I ran across this while building vorbis on the Mac:
In libvorbis RC2, file info.c, line 65, there's a call to strdup().
strdup() isn't portable, and it looks like the code should use the
internal memory functions to create the memory for the string anyway.

Actually, on further inspection, it looks like the memory for the
user comments is never actually freed.

I'm not subscribed to the list, so please CC me if you reply.

Thanks! :)
        -Sam Lantinga, Software Engineer, Blizzard Entertainment

--- >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@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2001-12-18 at 17:23:48 Sam Lantinga wrote:

SL> Hey, I ran across this while building vorbis on the Mac:
SL> In libvorbis RC2, file info.c, line 65, there's a call to strdup().
SL> strdup() isn't portable, and it looks like the code should use the
SL> internal memory functions to create the memory for the string anyway.

trdup() might not be portable, but almost every compiler in the world
has it, except MetroWerks CodeWarrior, both on Windows and Mac. And
even they have it somewhere, only they call it _strdup, for reasons
unknown to me.

Please try the attached patch for the vorbis component. This contains
a define for strdup (it makes sense to use the compiler intrinsic
function instead of a self-baked version, because it can be better
optimized), and a fix for the signedness problems in struct
vorbis_look_psy (which causes CW6 & 7 to complain).

Note that this is only for RC2, this patch might not work for the
head revision. But you shouldn't be using that for any production work
anyway. :) Please contact me if you need a patch for the main branch.

SL> Actually, on further inspection, it looks like the memory for the
SL> user comments is never actually freed.

That's another problem of course, can you please point out where, or
send a (unified) diff?

Cheers,
- --
Dimitry Andric <dim@xs4all.nl>
PGP Key: http://www.xs4all.nl/~dim/dim.asc
Fingerprint: 7AB462D2CE35FC6D42394FCDB05EA30A2E2096A3
Lbh ner abj va ivbyngvba bs gur QZPN

-----BEGIN PGP SIGNATURE-----
Version: PGP 6.5i
Comment: http://www.gn.apc.org/duncan/stoa_cover.htm

iQA/AwUBPB9wX7BeowouIJajEQKTtACg+NZRmLNP3vXfSte3QT8F4SmABTQAoJI/
RVanDTOjeYV+W3xUStV14rwj
=Ju5V
-----END PGP SIGNATURE-----

<HR NOSHADE>
<UL>
<LI>application/octet-stream attachment: vorbis.patch
</UL>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vorbis.patch
Type: application/octet-stream
Size: 895 bytes
Desc: not available
Url :
http://lists.xiph.org/pipermail/vorbis-dev/attachments/20011218/c22bd229/vorbis-0001.obj

> strdup() might not be portable, but almost every compiler in the world
> has it, except MetroWerks CodeWarrior, both on Windows and Mac. And
> even they have it somewhere, only they call it _strdup, for reasons
> unknown to me.
Actually, I was building with MPW, which isn't addressed in the patch.
The real problem is that strdup() calls malloc() internally, and the
vorbis library is neither freeing the strings returned by strdup(),
nor using the application provided memory functions to manage them.

It doesn't matter for me, since I already patched the code locally to
use the ogg memory functions, but I thought you would want to know.

Again, I'm not subscribed, so please CC me on replies.

See ya,
        -Sam Lantinga, Software Engineer, Blizzard Entertainment

--- >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@xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is
needed.
Unsubscribe messages sent to the list will be ignored/filtered.