Passing filename - as inputted string - to fopen() function

This is a discussion on Passing filename - as inputted string - to fopen() function within the C Programming forums, part of the General Programming Boards category; I figured this might as well be in its own thread. I'm surprised how I can't find a solution to ...

  1. #1
    Registered User
    Join Date
    Sep 2008
    Location
    Nort East of Great Britain!
    Posts
    8

    Passing filename - as inputted string - to fopen() function

    I figured this might as well be in its own thread.

    I'm surprised how I can't find a solution to this problem on the internet. It must be a very common error:

    Here is the code in question:


    Code:
        
       #define IBS 150
     
       ...
    
       int main() {
    
       /* Get Filename from user */
       char szbuf[IBS];
       puts ("Enter file to be opened: ");
       
       fgets(szbuf, IBS, stdin );
       char filename[ strlen(szbuf)];
       strcpy(filename, szbuf);
        
       /* Open File */  
       FILE * fp = fopen(filename, "r");
       
       ...
    I always get an error involving "constant char".

    I'm doing something wrong in what I am passing to the "fopen()" function. What does it want? I thought it was a string?

    Thanks in advance,

    Clint

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,457
    What is the error message?

    By the way, filename is a variable length array. Is that intended?
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Registered User
    Join Date
    Apr 2008
    Posts
    14
    One problem I see is in your input; fgets takes in the newline character you enter at the end. I deal with this by doing :

    Code:
    szbuf[strlen(szbuf) - 1] = 0; //Nullfies newline

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by epboyd View Post
    One problem I see is in your input; fgets takes in the newline character you enter at the end. I deal with this by doing :

    Code:
    szbuf[strlen(szbuf) - 1] = 0; //Nullfies newline
    Except that if you do that, and the line was longer than the capacity given, you will overwrite the last character. If instead you do
    Code:
    char *newlineptr;
    ...
    newlineptr = strchr(szbuf, '\n');
    if (newlineptr != NULL) // We found a newline
       *newlineptr = 0;
    // Potentially have an "else" to deal with lines that are too long to fit in the buffer.
    then we do not overwrite something that isn't a newline (and the difference in performance between strchr() and strlen() is marginal, in case anyone wonders).

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,452
    > I always get an error involving "constant char".
    As laserlight hinted, C doesn't have variable length arrays.
    Nor does it have mixed declarations and statements.
    If you dance barefoot on the broken glass of undefined behaviour, you've got to expect the occasional cut.
    If at first you don't succeed, try writing your phone number on the exam paper.
    I support http://www.ukip.org/ as the first necessary step to a free Europe.

  6. #6
    Registered User
    Join Date
    Oct 2007
    Posts
    100
    I'd replace
    Code:
    char filename[ strlen(szbuf)];
    with something like
    Code:
    char *filename;
    size_t len=strlen(szbuff);
    filename=(char *)malloc(sizeof(char)*(len+1)); //sizeof(char)=1, i put just to warn if you have to deal with other types
    if (filename==NULL)
    {
    printf("Out of memory\n");
    return 1;
    }
    Does the rest of the community agree?

  7. #7
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by smoking81 View Post
    I'd replace
    Code:
    char filename[ strlen(szbuf)];
    with something like
    Code:
    char *filename;
    size_t len=strlen(szbuff);
    filename=(char *)malloc(sizeof(char)*(len+1)); //sizeof(char)=1, i put just to warn if you have to deal with other types
    if (filename==NULL)
    {
    printf("Out of memory\n");
    return 1;
    }
    Does the rest of the community agree?
    Two aspects I'd like to point out:
    First: Why have two variables?
    There is szbuf already, what's the point of a second one called filename. Ok, it may be better to change szbuf to filename for naming practices, but I don't see any reason to copy it around.

    Second, if you must dynamically allocate the space, don't cast malloc() - this is the C section, not C++. Casting malloc is bad, and it's explained in the FAQ why it is so.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,457
    Personally, I think this would suffice:
    Code:
    /* Get Filename from user */
    char filename[IBS];
    FILE *fp = NULL;
    
    puts("Enter file to be opened: ");
    fgets(filename, IBS, stdin);
    
    /* Open File */
    fp = fopen(filename, "r");
    IBS is not a very descriptive name for the size of a filename though. Perhaps it stands for "input buffer size", but it leaves one to guess.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by laserlight View Post
    Personally, I think this would suffice:
    Code:
    /* Get Filename from user */
    char filename[IBS];
    FILE *fp = NULL;
    
    puts("Enter file to be opened: ");
    fgets(filename, IBS, stdin);
    
    /* Open File */
    fp = fopen(filename, "r");
    IBS is not a very descriptive name for the size of a filename though. Perhaps it stands for "input buffer size", but it leaves one to guess.
    Yes, that's what I was trying to say. The string may need cleaning of the newline at the end, but other than tat, it's fine.

    And IBS to me stands for Irritable Bowel Syndrom, but I can accept that is not what the original poster meant. Longer names are better [and in the case of a filename, perhaps MAX_PATH or PATH_MAX is a better choice - at least that stands a chance of holding a full path to some file deep down in the system. [Although for non-Windows, that solution CAN go wrong - but it's a better choice than some arbitrarily selected manual size].

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  10. #10
    Registered User
    Join Date
    Sep 2008
    Posts
    13
    I had this same problem a few weeks ago trying to open a text file.

    Just thought I'd add a reply to this aswell. My code assumes the person doesn't know the file extension and adds on ".txt" but you can change that to anything you like.


    Code:
    #define LNAME  	"C:\\Documents and Settings\\"              /* loadfile location */
    
    int load()
    {	
         int i;
         int len;	
    
         char file_name[50];
         char load_location[50] = LNAME;
    
         printf(" Enter file name: ");
         fflush(stdin);
         
         fgets(file_name, sizeof(file_name), stdin);
    
              /* remove newline left by fgets */
    
         len = strlen(file_name);
         if (file_name[len-1] == '\n') 
         file_name[len-1] = 0;
         
    			
         strcat(load_location, file_name);
         strcat(load_location, ".txt");
    	
         FILE *file = fopen(load_location, "r");
    }
    Last edited by george_1988; 09-22-2008 at 02:07 PM.

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,457
    You do not need a loop to remove the newline; strlen() already performs the loop, so you can use its return value to check the last character in the string.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  12. #12
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
         for(i = 0; i < strlen(file_name); i++)
              if(file_name[i] == '\n')
                   file_name[i] = '\0';
    This is VERY bad coding style, calling strlen() over and over in the loop. The length only changes when you get to the very end of the string, when you reach the '\n'. You can simplify it by just doing :
    Code:
    len = strlen(file_name);
    if (file_name[len-1] == '\n') 
       file_name[len-1] = 0;
    I make a separate variable for storing the length, so that we don't have to call strlen twice.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  13. #13
    Registered User
    Join Date
    Sep 2008
    Posts
    13
    Woops...

    Yeah I usually do something like:

    Code:
    len = strlen(file_name);
    if (file_name[len-1] == '\n') 
       file_name[len-1] = 0;
    When I'm writing code I have a habit of just doing "what works" for the first time. I usually go over my code when I'm ready to optimise it. Bad habit I can't get out of.
    Last edited by george_1988; 09-22-2008 at 02:06 PM.

  14. #14
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Quote Originally Posted by george_1988 View Post
    Woops...

    Yeah I usually do something like:

    Code:
    len = strlen(file_name);
    if (file_name[len-1] == '\n') 
       file_name[len-1] = 0;
    When I'm writing code I have a habit of just doing "what works" for the first time. I usually go over my code when I'm ready to optimise it. Bad habit I can't get out of.
    That's not a bad habit, however, "avoiding pessimisation" is a good thing.... A big warning light goes up when I see strlen in a loop-control, and in this case, it's definitely not meaningfull.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  15. #15
    a_capitalist_story
    Join Date
    Dec 2007
    Posts
    2,649

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Passing an array into a function
    By Ryston in forum C++ Programming
    Replies: 4
    Last Post: 08-29-2006, 05:20 AM
  2. Replies: 28
    Last Post: 07-16-2006, 11:35 PM
  3. Dikumud
    By maxorator in forum C++ Programming
    Replies: 1
    Last Post: 10-01-2005, 06:39 AM
  4. Calculator + LinkedList
    By maro009 in forum C++ Programming
    Replies: 20
    Last Post: 05-17-2005, 12:56 PM
  5. Passing string to function
    By PotitKing in forum C Programming
    Replies: 8
    Last Post: 05-24-2002, 05:17 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21