tags:

views:

263

answers:

6

Hello,

linux gcc c99

I am wondering what would be the best technique. To change a string. I am using strstr();

I have a filename called "file.vce" and I want to change the extension to "file.wav".

Is this the best method:

char file_name[80] = "filename.vce";
char *new_file_name = NULL;
new_file_name = strstr(file_name, "vce");
strncpy(new_file_name, "wav", 3);
printf("new file name: %s\n", new_file_name);
printf("file name: %s\n", file_name);

Many thanks for any advice,

================= I have edited my answer using using your suggestions. Can you see anything else wrong?

/** Replace the file extension .vce with .wav */
void replace_file_extension(char *file_name)
{
    char *p_extension;

    /** Find the extension .vce */
    p_extension = strrchr(file_name, '.');
    if(p_extension)
    {
     strcpy(++p_extension, "wav");
    }
    else
    {
     /** Filename did not have the .vce extension */
     /** Display debug information */
    }
}

int main(void)
{
    char filename[80] = "filename.vce";

    replace_file_extension(filename);

    printf("filename: %s\n", filename);
    return 0;
}
+1  A: 

You have to manually add zero-terminator at the end of new_file_name because strncpy() won't add it in your case.

It just happened that you already have zero byte at right place but it cannot be guaranteed in all situations so it is a good habit to do things like

new_file_name[3] = '\0';

after strncpy().

Also beware that string "vce" may appear inside filename.

qrdl
All actions are done inplace, the original null-terminator is still there.
sharptooth
I know. I'm just said it is a good habit to add the terminator manually. "stncpy(foo, bar, x); foo[x] = '\0';" is C idiom.
qrdl
At least it's a good habit to remember about this problem.
sharptooth
That was my point, exactly.
qrdl
+3  A: 

There are a few problems with:

char file_name[80] = "filename.vce";
char *new_file_name = NULL;
new_file_name = strstr(file_name, "vce");
strncpy(new_file_name, "wav", 3);
printf("new file name: %s\n", new_file_name);
printf("file name: %s\n", file_name);

There is only storage for one string, but at the end you attempt to print two different strings.

The variable named new_file_name actually points to part of the same filename.

The string vce might occur anywhere within a filename, not just an the extension. What if the filename was srvce.vce?

You probably want to find the last . character in the string, then check whether it is followed by the expected extension, and then replace that extension. And remember that if you do this by modifying the original buffer, you will not be able to print the old string afterwards.

Daniel Earwicker
+1  A: 

There are a few issues with your code. Here's a different take, with the changes commented inline:

char file_name[80] = "filename.vce";
char *new_file_name = NULL;

printf("old file name: '%s'\n", file_name);
/* Use strrstr(), to find the last occurance, and include the period. */
new_file_name = strrstr(file_name, ".vce");
if(new_file_name != NULL)
{
  /* Use plain strcpy() to make sure the terminator gets there.
   * Don't forget the period.
  */
  strcpy(new_file_name, ".wav");
}
printf("new file name: '%s'\n", file_name);

This can still be improved, it doesn't check that there is enough room to include the new extension, for instance. But it does terminate the string, in a slighly more natural fashion than poking at single characters.

unwind
This still has the misleading printout at the end. There's only one buffer here - how can it print the old and new filenames?
Daniel Earwicker
Very true, thanks for pointing that out. I've edited my answer. I guess I focused mainly on the string operations.
unwind
+1  A: 

I would do this

char file_name[80] = "filename.vce";
char *pExt;

pExt = strrchr(file_name, ".");
if( pExt )
  strcpy(++pExt, "wav");
else
{
 // hey no extension
}
printf("file name: %s\n", file_name);

You NEED to do pointer manipulation in every c program. Of course you'd do some more checking for buffer over runs etc. or even use the path specific functions.

Martlark
+2  A: 

Rather than search for . or vce either of which may appear multiple times in the string, calculate the length of the string and subtract 3 to point to the extension. Replace the extension in-place using strncpy.

size_t length;
char* pextension;
char file_name[80] = "filename.vce";
printf("file name: %s\n", file_name);    
length = strlen(file_name);
pextension = file_name + length - 3;
strncpy(pextension, "wav", 3);
printf("new file name: %s\n", file_name);
shouston
+1  A: 

I'm bored, and instead of pointing out problems in the original code, I wrote my own. I tried to keep it clear for instructive purposes.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>


/* Replace the last suffix in a filename with a new suffix. Copy the new
   name to a new string, allocated with malloc. Return new string. 
   Caller MUST free new string.

   If old name has no suffix, a period and the new suffix is appended
   to it. The new suffix MUST include a period it one is desired.

   Slashes are interepreted to separate directories in the filename.
   Suffixes are only looked after the last slash, if any.

   */
char *replace_filename_suffix(const char *pathname, const char *new_suffix)
{
    size_t new_size;
    size_t pathname_len;
    size_t suffix_len;
    size_t before_suffix;
    char *last_slash;
    char *last_period;
    char *new_name;

    /* Allocate enough memory for the resulting string. We allocate enough
       for the worst case, for simplicity. */
    pathname_len = strlen(pathname);
    suffix_len = strlen(new_suffix);
    new_size = pathname_len + suffix_len + 1;
    new_name = malloc(new_size);
    if (new_name == NULL)
        return NULL;

    /* Compute the number of characters to copy from the old name. */
    last_slash = strrchr(pathname, '/');
    last_period = strrchr(pathname, '.');
    if (last_period && (!last_slash || last_period > last_slash))
        before_suffix = last_period - pathname;
    else
        before_suffix = pathname_len;

    /* Copy over the stuff before the old suffix. Then append a period
       and the new suffix. */
#if USE_SPRINTF
    /* This uses snprintf, which is how I would normally do this. The
       %.*s formatting directive is used to copy a specific amount
       of text from pathname. Note that this has the theoretical
       problem with filenames larger than will fit into an integer. */
    snprintf(new_name, new_size, "%.*s%s", (int) before_suffix, pathname,
             new_suffix);
#else
    /* This uses memcpy and strcpy, to demonstrate how they might be
       used instead. Much C string processing needs to be done with
       these low-level tools. */
    memcpy(new_name, pathname, before_suffix);
    strcpy(new_name + before_suffix, new_suffix);
#endif

    /* All done. */
    return new_name;
}


int main(int argc, char **argv)
{
    int i;
    char *new_name;

    for (i = 1; i + 1 < argc; ++i) {
        new_name = replace_filename_suffix(argv[i], argv[i+1]);
        if (new_name == NULL) {
            perror("replace_filename_suffix");
            return EXIT_FAILURE;
        }
        printf("original: %s\nsuffix: %s\nnew name: %s\n",
               argv[i], argv[i+1], new_name);
        free(new_name);
    }
    return 0;
}
Lars Wirzenius