strcopy() why doesn't this work?

[Dan Feerst]

So I've never been 100% clear on character arrays vs strings but I'm still uncertain why this doesn't work. I'm trying to write a function to get the extension of a file used as a command line argument and put it in a character array. The function needs to be able to deal with extensions with lengths other than three characters.

The error says that it can't convert a char to const char*

I don't know why the array I declared is considered constant.

int main(int argc, char *argv[])
{

    int len= strlen(argv[1]);
    int dot;
    for(int i=0;i <=( len-1); i++){
    if(argv[1][i]=='.') dot=i;
    }
    int extlen= (len-1) - dot;
    char ext[4];
    strcpy (ext,argv[1][dot+1]);
    for(int i = (dot + 2); i <= (len-1); i++){
      strcat(ext,argv[1][i]);
    }
    puts (ext);
    cout << ext<<endl;
    return 0;

Obviously, this is only part of the code, but I need to get this working first.

[Zyoma]

Try this:

strcpy (ext,&argv[1][dot+1]);

and also for strcat

strcat(ext,&argv[1][i]);

You need to copy a string, not only one character. So, you should use an address from which the extension begins.

[webJose]

The correct statement is:

strcpy(ext, argv[1] + dot + 1);

But this code has a major problem: What if the extension is larger than 3 characters? I don't know which OS you are programming for, but if it is for Windows you'll find plenty of file extensions larger than 3 characters. The function above will corrupt the stack in those cases: A typical buffer overrun.

What you need to do is either dynamically allocate the extension char array.

Regardless of what you do, you should stop using strcpy() for production code because of this buffer overrun risk. Insted you should always use strncpy() to avoid this risk:

strncpy(ext, argv[1] + dot + 1, std::min(sizeof(ext) - 1, extlen));

The above uses std::min() to return the minimum of the two values: The size of memory I have available or the size of the extension. This way you never overrun your buffer. BTW, std::min() requires #include <algorithm>.

[kbw]

If the extension starts at the last '.', you find it with strrchr(). It returns NULL if the char isn't found.

#include <iostream>
#include <cstring>

int main(int argc, char* argv[])
{
    for (int i = 1; i < argc; ++i)
    {
        if (const char* p = strrchr(argv[i], '.'))
        {
            // argv[i] is the start of the buffer
            // p is where the last dot is
            // bodylen is the length of name without extension
            // extlen is the length of the extension (with the dot)
            size_t bodylen = p - argv[i];
            size_t extlen = strlen(p);

            // make up a new string to hold the name without extension
            char* body = new char[bodylen + 1];
            strncpy(body, argv[i], bodylen);
            body[bodylen] = 0; // add null terminator

            // make up a new string to hold the extension
            char* ext = new char[extlen + 1];
            strcpy(ext, p);

            // do something with them
            std::cout << "body=" << body << ", ext=" << ext << std::endl;

            // cleanup
            delete [] ext;
            delete [] body;
        }
    }

    return 0;
}

[Dan Feerst]

Thanks everyone. I think I fixed it.
I haven't been able to break it.

I didn't end up going with strncpy, I can see how it would be
useful, and I'm glad you pointed it out because I didn't know about that
function, but I think the line

char ext[extlen+1];

prevents any possibilities of overflow. Please tell me if I'm wrong, but I haven't been able to break it yet.

int main(int argc, char *argv[])
{
    if(argc>1){
        int len= strlen(argv[1]);
        int dot=len-1;
        for(int i=0;i <=( len-1); i++){
            if(argv[1][i]=='.') dot=i;
            }
        int extlen= (len-1) - dot;
        char ext[extlen+1];
        strcpy (ext,&argv[1][dot+1]);
        cout<<ext<<endl;
    }
    return 0;

[webJose]

That's not standard C++ and you should avoid it at all costs. The only standard way of allocating memory using a variable is through the use of operator new(). Using char ext[extlen + 1]; is a non-standard extension that should never be used because you never know which compiler you may end up using, and I think there's only one compiler out there that allows this anyway.

So Yes, that ensures you won't overrun your buffer, but it is not standard C++. You should abide by the standard and allocate memory using operator new().

[modoran]

Well, if this is a C++ program, you don't even need to allocate any memory at all. std::string class manages all of this for you and it has methods to get last occurence of a character in a string, as it is in your case, so no need for strcpy, strlen or whatever. Using operator new in this program is absolutely not required/recommended at all.

If this is a C program use malloc to allocate memory, keeping your existing code.