#40 change output file extension adding

[bgb/cr88192 null]

I found it annoying that, when outfile is ommited, the
generated name was just the input name with '.mp3'
tacked on.

eg:
lame foo.wav
names output foo.wav.mp3 instead of foo.mp3

ok, in the version I had I tweaked this:
frontend/parse.c:1901
char *sp, *spb;
spb=outPath;
sp=spb+strlen(spb);
while((sp>spb) && (*sp!='.'))sp--;
strcpy(sp, ".wav");
vs:
strncat (outPath, ".wav", 4 );

similar went for mp3, located just below.

reasoning is that supplying an output name is not
allways a possibility (say, when adding it as a windows
explorer command), and renaming would be a hassle (eg:
when converting a bunch of files at once).

maybe consider an option to unlink the input file post
conversion (eg: like gzip).

or such...

[Nobody/Anonymous]

Logged In: NO

This should be wrapped in

if (strchr(outPath, ".")) {
...
} else {
strncat(outPath, ".wav", 4);
}

There's a problem with the use of strncat there. It's no
different than strcat(outPath, ".wav") because the third
parameter is the number of bytes to copy. We know there
will always be exactly four bytes to copy. The problem is
(and I think this is what the code was trying to prevent)
that the outPath buffer might not have room for four more
bytes.

You have to do the call like this:

strncat(outPath, ".wav", OUTPATHSIZE-strlen(outPath)-1);

Note that the resulting third parameter can be more than
four. That is ok. strncat will not copy past the NUL
terminator in the string constant.

A problem remains if there is no room... copying zero
characters leave outPath unchanged and the original file
will be overwritten.

Another choice is to do something like:

if (strlen(outPath)+5 < OUTPATHSIZE)
strcat(outPath, ".wav");
else
strcat(outPath+OUTPATHSIZE-5, ".wav"); // stuff .wav into
the last part of the buffer, overwriting part of the
filename... not nice but there is no better option

[bgb/cr88192 null]

Logged In: YES
user_id=129948

from what I remember I didn't see anything trying to avoid
overflowing the buffer.
maybe I am just lazy, but often I don't really worry that much
about buffer overflows (yes, they can crash the app, but the
buffer is big enough that the risk is not too major).

I mostly changed it (in my version at least) because the
names being generated were stupid...

why generate: "foo.wav.mp3" when "foo.mp3" makes more
sense anyways?...

that use of strncat was there originally, imo it seemed fairly
pointless, but whatever (I commented it out, and added in my
own bit, which was the slightly longer fragment that occured
previously).

my comment was mostly to show about where and what I had
changed, possibly saving others for having to spend the
several minutes or whatever it would take to find it on their
own...

in general your comment was difficult to decipher though.

[Nobody/Anonymous]

Logged In: NO

I agree with you that your change is a good idea.

I was just pointing out that you added a new buffer overflow
(not a huge concern as you say because it is a local program
and not suid, but still) PLUS there was already an existing
problem. If nobody is concerned about the buffer overflows
then I suggest just using strcat(outPath, ".wav") instead as
it does exactly the same thing plus it is more readable.

My other point was that if the original file didn't have a
"." in it, your algorithm would change the filename to just
".wav" which isn't very nice, especially for multiple input
files without dots in their names. A better approach would
be to use the original algorithm if there is no ".".

Suggested code sequence:

char *sp, *spb;

spb = outPath;
/* OUTPATHSIZE should be sizeof(outPath) if it
* is an array, otherwise the number of bytes in outPath.
* It should be at least 5 otherwise more checks are
* required below.
*/
if (strchr(outPath, ".")) {
// replace existing extension
sp = spb+strlen(spb);
while ((sp>spb) && (*sp!='.')) sp--;
strncpy(outPath, ".wav", OUTPATHSIZE-(sp-spb)-1);
} else {
//
if (strlen(outPath)+5 < OUTPATHSIZE) {
strcat(outPath, ".wav"); // safe: we know there is room
} else {
strcpy(outPath+OUTPATHSIZE-5, ".wav"); // overwrite
part of the name at the end because there is no room
}
}

Side note: The interface to strncpy() is horrible.

[Gabriel Bouvigne]

Logged In: YES
user_id=1056

You also have to remember that Lame accepts mp3 files as
input, so the input file name might already have an *.mp3 name.

[bgb/cr88192 null]

Logged In: YES
user_id=129948

yes, this could be a problem. this could be fixed though, or
names cound be given manually in this case.

I am mostly just thinking that windows explorer is fairly
stupid, so having it come up with the target name is not
possible afaik, this is part of why I did it (I could have just
used the shell, but whatever).

[Robert Hegemann]

When not specifying an output filename, LAME 3.98 replaces some known input file extensions by ".mp3". If input file extension is the same as the output file extension, LAME behaves as previously and appends ".mp3". (Same for decoding and ".wav" extension)

[Robert Hegemann]

Ups. This feature isn't in 3.98 but introduced in 3.99.