Renaming a file with a variable as the input name

Hi all, I'm trying to rename a file, based on what the user wants to save it to. The file is called simply "temp", and is deleted when the program exits, but if the user wants to save the progress they can choose to save it with their own filename. Here is my code snippet, it always jumps to "Error saving file".

Code:

---

char saveName[20];
char accept;
int renameFile = 0;
               
printf("\nSave file to  : ");
scanf("%s",saveName);
strcat(saveName, ".txt");
printf("Filename is    : %s\n",saveName);
printf("Accept? Type Y for yes or N for no : ");
scanf("%s",&accept);
                       
if(accept == 'y' || accept == 'Y')
{
        renameFile = rename("temp",saveName);
                               
        if(renameFile != 0)
        {
                printf("\nError saving file.\n\n");
        }
}
else if(accept == 'n' || accept == 'N')
{
        printf("\nFilename rejected, progress will not be saved.\n\n");
}
else
{
        printf("\nInvalid choice, progress will not be saved.\n\n");
}

---

Most likely because the %s from scanf will have a '\n' at the end (the newline from the enter key), which is not a valid character in a filename.

You need to do some "input validation". There are two approaches. One is blacklisting, where you get your string, and then deleted unwanted characters. The other is whitelisting, where you delete everything that is not a wanted character. So a blacklist contains unwanted characters (eg, the '\n') whereas a whitelist contains desirable characters.

Blacklisting is used when there are a few special characters you want to eliminate because they could be used maliciously to sabotage code. But whitelisting is better for filenames because filenames are restricted to a predefined set of characters. Also, (fortunately) whitelisting is somewhat easier in C using scanf:

Code:

---

scanf("%[a-zA-Z0-9_-+]",saveName);

---

That will pick a string IF it contains only a-z, A-Z, 0-9, underscore, or the dash, or plus. It will also pull a sting "in so far" as it contains those, so:

myfile_1///\n

will be "myfile_1". Note, this means you leave on the newline. Yay! However, this

// ///my//file ///\n

will probably just be "my" since that is the first acceptable sequence (get it?).

You should check the return value of scanf to ensure that a string was in fact, aquired, since this

/~?/ ///} }}\n

will fail to produce anything. A final note: you probably also want to limit the length of the name:

Code:

---

scanf("%16[a-zA-Z0-9_-+]",saveName);

---

Hmm, thanks for the info but with those changes applied it doesn't allow me to even enter a filename, it jumps straight to printing out a load of random characters in the "Filename is" printf statement. With the original code, when it prints out the filename entered, it is the correct filename, ie, if i enter test it will print out "Filename is : test.txt"

Quote:

---

Originally Posted by cherryduck

Hmm, thanks for the info but with those changes applied it doesn't allow me to even enter a filename, it jumps straight to printing

---

Hmm. Well, check the return value of scanf to see if it has read something.

A definite issue will be that anything AFTER the string that is not used will be left in the input buffer. This will be the first thing used in the next scanf, BEFORE it processes anything fresh entered at the keyboard. For example:

Code:

---

printf("Enter data: ");
scanf("%[a-z]",string1);
printf("%s\n",string1);
printf("Enter data: ");
scanf("%s",string2);
printf("%s\n",string2);

---

If, at the first prompt, I type:

asdfXX\n

What you will see happen is this;

Enter data: asdfXX
asdf
Enter data:
XX

Ie, the user will not get a chance to enter anything at the second prompt. You can take care of that by using * to discard an item in a scanf:

Code:

---

scanf("%[a-z]%*s",string1);

---

This will put the "a-z" limited string into string1 and discard anything after that, including the newline which is inevitably there.

Try to think about that complication, then if things are still screwed up post the code you have currently and I can compile and test it.

Still getting nowhere I'm afraid :( I'll post all my code as it is. There are several other issues that need fixing as well if you're feeling particularly helpful :p

Also, I'm supposed to be building this to C89/C90 standards, so it needs to be compiled with the -ansi flag. I'm currenly using getopt.h which I know isn't in the original standard so I'll fix that up with some if statements later.

I also realise my code is probably pretty awful, but I'm a C newbie so I'm just trying to make something that works first, before I look at optimisations and properly allocating memory and passing variables etc.

Quote:

---

Originally Posted by cherryduck

Still getting nowhere I'm afraid :( I'll post all my code as it is. There are several other issues that need fixing as well if you're feeling particularly helpful :p

---

If you point out specific issues I or someone may have time to look (eg, I did not read your todo.txt...I have one of my own ;))

Anyway, my solution with chucking a string at the end of the scanf did not work so well in this circumstance. So I wrote this based on what you have around line 166. It should be the real foolproof method:

Code:

---

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

int getfilename(char *name) {
        int r;
        char buffer[128];
        printf("\nSave file to  : ");
        fgets(buffer,128,stdin);
        r=sscanf(buffer,"%15[a-zA-Z0-9_%-.=+]",name);
        return r;
}

int main(void) {
        int r;
        char saveName[20];
        char accept;
       
        while (!r) r = getfilename(saveName);
        strcat(saveName, ".txt");
        printf("Filename is    : \"%s\"\n",saveName);
        printf("Accept? Type Y for yes or N for no : ");
        scanf("%c",&accept); /* notice %c not %s! */
        printf("\nThe answer was ->%c<-\n",accept);
       
        return 0;
}

---

That compiles and runs -ansi, try it to see how it works. The comment in red is VERY IMPORTANT as otherwise you will be GUARANTEED to create a buffer overflow, which could account for other subsequent problems (including potential seg faults). Currently, you are making that mistake! Also, this still leaves a newline in the input buffer, which does not matter in the context of this code but it may in yours...

Notice I used quotes around the variables in printf, which helps indicate when there is a newline on the end -- you will get:

The filename is: "myfile
"

Altho with the above code that will not happen. Writing short, specific, potentially recyclable routines like "getfilename" is a good way to work, BTW. It also helps keep your code organized and comprehensible.

Cheers for all the help, it's greatly appreciated! And your avatar really confused me...