Thread: can u find any bug in this simple code??

  1. #1
    Registered User
    Join Date
    May 2010
    Posts
    21

    Question can u find any bug in this simple code??

    Hi all,
    I'm newbie to C and i'm trying do a simple program for adding records into a text file using file streams. I'm using gets() to get user name with spaces i dont know y i'm not about to use that gets() function properly.Could anyone tell what exactly the problem with this code... thanks in advance...

    Code:
    #include<stdio.h>
    #include<stdlib.h>
    #include<stdbool.h>
    
    struct employee
    {
       char name[150];
    };
    
    int main()
    {
     addRecord();
     getchar();
     return 0;
    
    }
    
    int addRecord()
    {
      FILE *fp;
      struct employee emp[1000];
      char ch;
      int i=0,count=0;
      fp = fopen("E:\\SRI\\C_Projects\\tel_Directory_struct\\intoFile.txt","w");
      if(fp!= NULL)
      {
       printf("enter how many records u want to add??");
       scanf("%d",&count);
       for(i=0;i<count;i++)
       {
         printf("Enter the Name of the employee:");
         gets(emp[i].name);  // here problem encounters and not able to receive  
                                         //  user input,program is terminating...:(
         fprintf(fp,"%s\n",emp[i].name);
       }
       fclose(fp) ;
       getchar();
       return 0;
      }
      else
      {
        printf("File couln't open...");
        exit(1);
      }
    
    }

  2. #2
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Well, one problem is that scanf("%d",&count) leaves the newline in the input buffer, so the first gets() call reads that line in. What you should do is to read and discard characters until the newline character is discarded, and only then do you read in the actual first line.

    But you should not be using gets() to begin with, since it does not provide you with any way to limit the number of characters entered so as to avoid buffer overflow. What you should use in this case is fgets().
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  3. #3
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    You haven't told us whether to look for a compile-time problem, or a run-time problem. Also, in either case, you have to give the details about what the actual symptoms or errors are.

    So I declare that the "problem" is that your main definition should be:
    Code:
    int main(void)
    not merely
    Code:
    int main()
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  4. #4
    Novice
    Join Date
    Jul 2009
    Posts
    568
    Code:
    struct employee emp[1000];
    With this you're creating an array for 1000 employees. But the array does not actually contain any employees. You're just asking for enough memory to store them, and that's what you're getting.

    Think of it like this. You bought a bookshelf to hold 1000 books, but you didn't put any books into it. Now you're trying to pick one book from the shelf and read it. But there are no books! Demons fly out of your nose!

    Put some books on the shelf.

    (Well. Your code is more like trying to make a note in the book. But that's besides the point.)
    Last edited by msh; 05-26-2010 at 01:20 AM.

  5. #5
    Registered User
    Join Date
    Jan 2009
    Location
    Australia
    Posts
    375
    There's no function declaration of addRecord() before main(), so the compiler won't know where to look for it.

  6. #6
    Registered User
    Join Date
    Jan 2009
    Location
    Australia
    Posts
    375
    Quote Originally Posted by msh View Post
    Code:
    struct employee emp[1000];
    With this you're creating an array for 1000 employees. But the array does not actually contain any employees. You're just asking for enough memory to store them, and that's what you're getting.

    Think of it like this. You bought a bookshelf to hold 1000 books, but you didn't put any books into it. Now you're trying to pick one book from the shelf and read it. But there are no books! Demons fly out of your nose!

    Put some books on the shelf.

    (Well. Your code is more like trying to make a note in the book. But that's besides the point.)
    He's adding the data to the structure just before it is written to the file.

    That said, if the user enters more than 999 for the number of employees, then you'll receive a buffer overrun.

    EDIT: The compiler may also give a warning/error because if the file pointer is NULL then there is no return value. I understand that you're using exit() to exit the application, but your compiler may not.
    Last edited by DeadPlanet; 05-26-2010 at 01:26 AM.

  7. #7
    Novice
    Join Date
    Jul 2009
    Posts
    568
    Quote Originally Posted by DeadPlanet View Post
    He's adding the data to the structure just before it is written to the file.

    That said, if the user enters more than 1000 for employees, then you'll receive a buffer overrun.
    What structure? There are no structures in emp[]; it's uninitialized.

  8. #8
    Registered User
    Join Date
    Jan 2009
    Location
    Australia
    Posts
    375
    It's uninitialised in that there is no meaningful data in the structures, but there are indeed one thousand structures in the array of 'employee' data structures.

  9. #9
    Registered User
    Join Date
    May 2010
    Location
    Naypyidaw
    Posts
    1,314
    What deathplanet said was right. I don't understand 'no structures in emp[]'???

  10. #10
    Registered User
    Join Date
    May 2010
    Posts
    21

    Question Hi...i undestood that but....

    hi laserlight,
    the problem is when i run the program i'm jus getting output as follows...
    o/p of the program is...

    enter how many records u want to add?? 2 (if i press Enter key here then)
    Enter the Name of the employee:Enter the Name of the employee:
    (and program terminates)

    even i changed gets(emp[i].name) to fgets(emp[i].name,150,fp) but still i'm getting the same output...
    what is the problem exactly...

    thanks in advance..
    sri


    Quote Originally Posted by laserlight View Post
    Well, one problem is that scanf("%d",&count) leaves the newline in the input buffer, so the first gets() call reads that line in. What you should do is to read and discard characters until the newline character is discarded, and only then do you read in the actual first line.

    But you should not be using gets() to begin with, since it does not provide you with any way to limit the number of characters entered so as to avoid buffer overflow. What you should use in this case is fgets().

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    28,413
    Quote Originally Posted by DeadPlanet
    There's no function declaration of addRecord() before main(), so the compiler won't know where to look for it.
    Yeah, though by "luck" the signature and return type of addRecord does not pose a problem for the default int misfeature.

    Quote Originally Posted by msh
    What structure? There are no structures in emp[]; it's uninitialized.
    No, there are 1000 struct employee objects in emp. It is true that they were not initialised, yet the objects were not used until after the respective name member was written to. That said, it certainly looks like only one struct employee object is needed, since the contents of each is immediately written to file and then not used.

    Quote Originally Posted by mgnidhi_3july
    the problem is when i run the program i'm jus getting output as follows...
    o/p of the program is...

    enter how many records u want to add?? 2 (if i press Enter key here then)
    Enter the Name of the employee:Enter the Name of the employee:
    (and program terminates)

    even i changed gets(emp[i].name) to fgets(emp[i].name,150,fp) but still i'm getting the same output...
    what is the problem exactly...
    Changing from gets to fgets fixes one problem, but you still need to fix the one concerning scanf: as I noted scanf leaves a newline in the buffer. This is read by the first gets (or now fgets) call, thus that call does not read the line that you want it to read.
    Quote Originally Posted by Bjarne Stroustrup (2000-10-14)
    I get maybe two dozen requests for help with some sort of programming or design problem every day. Most have more sense than to send me hundreds of lines of code. If they do, I ask them to find the smallest example that exhibits the problem and send me that. Mostly, they then find the error themselves. "Finding the smallest program that demonstrates the error" is a powerful debugging tool.
    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  12. #12
    Registered User
    Join Date
    May 2010
    Location
    Naypyidaw
    Posts
    1,314
    Read the c-faq

  13. #13
    Registered User
    Join Date
    Jan 2009
    Location
    Australia
    Posts
    375
    what is the problem exactly...
    The problem is exactly as laserlight stated in the second post of this very thread.

    Well, one problem is that scanf("%d",&count) leaves the newline in the input buffer, so the first gets() call reads that line in. What you should do is to read and discard characters until the newline character is discarded, and only then do you read in the actual first line.

  14. #14
    Novice
    Join Date
    Jul 2009
    Posts
    568
    Quote Originally Posted by DeadPlanet View Post
    It's uninitialised in that there is no meaningful data in the structures, but there are indeed one thousand structures in the array of 'employee' data structures.
    My mistake. I was drawing parallels between these two:
    Code:
    struct employee emp[1000];
    AND
    struct employee *emp = malloc( sizeof(*emp) * 1000);

  15. #15
    Registered User
    Join Date
    May 2010
    Posts
    21

    Question hi laser

    i'm newbiew to this c worls...unless u give me the explanation is simple words its quite difficult to understand.For this problem what exactly do i need to do...i used fgets() but still u r telling some more problem is remaining then how can get rid of that.. go easy on me if i upset u...

    thanks in advance...

    Quote Originally Posted by laserlight View Post
    Yeah, though by "luck" the signature and return type of addRecord does not pose a problem for the default int misfeature.


    No, there are 1000 struct employee objects in emp. It is true that they were not initialised, yet the objects were not used until after the respective name member was written to. That said, it certainly looks like only one struct employee object is needed, since the contents of each is immediately written to file and then not used.


    Changing from gets to fgets fixes one problem, but you still need to fix the one concerning scanf: as I noted scanf leaves a newline in the buffer. This is read by the first gets (or now fgets) call, thus that call does not read the line that you want it to read.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. True ASM vs. Fake ASM ????
    By DavidP in forum A Brief History of Cprogramming.com
    Replies: 7
    Last Post: 04-02-2003, 04:28 AM
  2. Very simple question, problem in my Code.
    By Vber in forum C Programming
    Replies: 7
    Last Post: 11-16-2002, 03:57 PM
  3. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM
  4. Replies: 1
    Last Post: 01-29-2002, 02:49 AM