2

I have been trying to create a simple program. However, I encountered an error:

gmon.out:too many open files

I am not clear on why it says I have "too many open files". It does not appear I am using files.

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

struct position
{
    int line;
    int place;
    struct position *next;
};
struct file
{
    struct position *info;
    struct file *next;
    char *name;
};
struct word
{
    char *name;
    struct word *right;
    struct word *left;
    struct file *result;
};

int main()
{
    int i;
    struct word *d,*c;
    char *s="brutus";
    printf("%s",s);
    c=(struct word*)malloc(sizeof(struct word));
    strcpy(c->name,s);
    c->left=NULL;
    c->right=NULL;
    for(i=1;i<=10;i++)
    {
        d=(struct word*)malloc(sizeof(struct word));
        if(d==NULL)
            exit(0);

        scanf("%s",s);
        printf("4");
        s=d->name;

        printf("%s",d->name);

        d->left=NULL;
        d->right=NULL;
    }
    system("pause");
    exit(0);
}

What should I do about it?Thank you in advnace for your time!

Engineer2021
  • 3,288
  • 6
  • 29
  • 51
user3697730
  • 251
  • 1
  • 3
  • 13
  • 1
    No allocation of `s` ... the memory will be corrupted when `scanf` is called. – Bentoy13 Jul 09 '14 at 13:51
  • 1
    @Al.Sal Please have a look at this post http://meta.stackexchange.com/questions/15650/ban-lmgtfy-let-me-google-that-for-you-links – cerkiewny Jul 09 '14 at 13:51
  • Important, yet unrelated remark: [Do not cast the return value of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858) – Elias Van Ootegem Jul 09 '14 at 13:53
  • 1
    I don't mind at all if people cast the return value of malloc. C++ developers do it all the time, so I'd want to hear a good reason what's bad about it, assuming a decent development system. – gnasher729 Jul 09 '14 at 13:56
  • I added s=(char*)malloc(20*sizeof(char)); but still,it doesn't work...Thank you for your time though...! – user3697730 Jul 09 '14 at 13:56
  • @gnasher729: Read the link. In C++, you _have_ to cast the pointer. C != C++, though. using `some_ptr = malloc(10 *sizeof *some_ptr);` is the C-way, if you will. You're free to change the type of `some_ptr`, without having to worry about the memory-management functions at all, simply because you're using `sizeof *some_ptr`. Think about custom allocators and such: `void * my_malloc(void **some_var, size_t size_type, size_t multiplier) { *some_var = realloc(*some_var, multiplier*size_type); if (*some_var == NULL) return null; return *some_var;}` try that without casts. – Elias Van Ootegem Jul 09 '14 at 14:03
  • 1
    @user3697730: `char *s` is actually a `const char *s`, because you've assigned a string literal: that string is stored in _read-only_ memory – Elias Van Ootegem Jul 09 '14 at 14:06
  • Well,it seems that you are right...!I added everwhere some mallocs and now it works...!Thanks a lot!!I had been trying to solve this problem for hours...!I am still a beginner at programming unfortunately...Willem M and gnasher729,you both helped a lot...For this reason,I cannot choose to vote one of your answeres..Thank you though! – user3697730 Jul 09 '14 at 14:17
  • From now on,I will be using mallocs everywhere,just to make sure that I won't face similar problems...Even when they are not needed! – user3697730 Jul 09 '14 at 14:18
  • 1
    @user3697730: Please read through the help section of this site: you're requested not to leave _"thank you"_ comments. Instead mark the answer that helped you the best as accepted. I've bundled all of my comments + more details into an answer, because these comments only addressed half of the issues you had – Elias Van Ootegem Jul 09 '14 at 14:26

4 Answers4

2

First off:

gmon.out:too many open files

Means that you're compiling with the -p flag (profiling). gmon.out is the default file-name used by gprof. Just ditch-the-switch, and you won't get that problem anymore.
Of course, not profiling code isn't great, but you'd do well to address a coupe of issues first, before setting about actually profiling your code.

Some of these, quite numerous, issues are:

char *s="brutus";
printf("%s",s);
c=(struct word*)malloc(sizeof(struct word));
strcpy(c->name,s);

List of issues:

  • char *s should be const char *s, because it points to read-only memory.
  • Next, Do not cast the return of malloc
  • Check the return value of functions like malloc, they tell you something
  • struct wordis a struct of which all members are pointers. After allocating the struct, those pointers are invalid: you need to allocate memory for those members, too
  • strcpy expects the destination (c->name) to be a valid pointer, as I explained above: this is not the case here

What, then, should this code look like:

const char *s = "brutus";
c = malloc(sizeof *c);
if (c == NULL)
{
    fprintf(stderr, "Could not allocate memory for struct word\n");
    exit( EXIT_FAILURE );
}
//allocate enough memory to store the string
c->name = malloc(
    (strlen(s)+1) * sizeof *c->name
);
//OR same, but shorter, works because the type char is guaranteed by the standard to be 1 byte in size
c->name = malloc(strlen(s)+1);
if (c->name == NULL)
    exit( EXIT_FAILURE );//could not allocate mem
c->name[0] = '\0';//set to empty string, now we can use safer functions:
strncat(c->name, s, strlen(s));

After you address these issues, seriously re-think your approach, and ask yourself what it is you're actually trying to do here:

for(i=1;i<=10;i++)
{
    d=(struct word*)malloc(sizeof(struct word));
    if(d==NULL)
        exit(0);

    scanf("%s",s);
    printf("4");
    s=d->name;
}

You're allocating a struct 10 times, each time re-assigning it to d. You never free this memory, though. which is bad practice.
Again: don't cast the return of malloc, but that's the least of your worries.

if (d == NULL)
    exit(0);

Ok, now you check the return of malloc. Great. But why on earth are you terminating with 0 (indicative of a successful run). There's a macro for this, too. You could've written:

if (d == NULL)
    exit( EXIT_SUCCESS);

Clearly, EXIT_SUCCESS is not what you should communicate.
that const char *s is now being used to store user input. That's not going to work, though, as it points to read-only memory, so forget about the unsafe scanf("%s", s); statement. Use a stack variable, and make sure the input buffer is cleared, or use a safe alternative.
But then you go and do something as absurd as this:

s = d->name;

Again, d->name, like in the case with c, is an invalid pointer. Why assign it to s here? there's no point, no reason... only madness.

Bottom line: Kill this code before it hatches, start again, and please use these tips/recommendations and critiques as a guideline.

Community
  • 1
  • 1
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
0

I have no idea why you're getting a 'too many open files', but this line:

strcpy(c->name,s)

is writing data to random memory, which could cause all kinds of problems. You need to malloc() that c->name first.

Also that scanf to s looks suspicious, and d->name is never assigned anything either.

The reason that you're getting 'too many open files' is probably because some memory is getting overwritten in such a way that just happens to trigger that particular error. Welcome to the world of undefined behaviour. IE: If you overwrite random memory, basically anything can happen.

Willem M.
  • 484
  • 5
  • 8
0

The first bug is in the line

    strcpy(c->name,s);

At that point, c->name is an uninitialised pointer so the program will crash if you are lucky.

Reading your comment: You fixed the second bug. The first bug is still unfixed. And there's the third bug in the line

    s=d->name;
gnasher729
  • 51,477
  • 5
  • 75
  • 98
0

This string copy will run off through memory, starting at whatever c->name points to until it finds a null terminator.

strcpy(c->name,s);

You have allocated space for c but not for the name pointer in c.

c->name = malloc([some length]);

c->name points somewhere, but you don't know where until you malloc it. That's why you're getting a seemingly random error, because your executing a string copy from an unknown location for an unknown number of bytes and you are clobbering whatever s points to for an unknown number of bytes.

nicomp
  • 4,344
  • 4
  • 27
  • 60