Solved

Linked List

Posted on 2000-05-05
10
219 Views
Last Modified: 2013-12-14
I'm created a link list of structures.  the struct is defined with:
           char name[50];
           char num[50];
I load this list with X number of names and numbers, but are char based.  Then later I go back in the list and search for a number and return the name.  Note the names are not the same length, but less than 50.  After 22 items have be written to and read from the list it returns a bad name.  It inserts "Off" and returns "O%", then it continues perfectly.  I've debugged everything and the insert goes fine but when it goes to read the "Off" back out its gets some garbage.  I thought this was in indication of a memory leak but I can't find one.  What do you think?

Here is some of the code I'm using.

struct enum_info {
   char enum_name[50];
   char enum_value[50];
   struct enum_info *next;
   struct enum_info *prior;
} enum_list;
char enum_name[50];
char enum_num[50];

l_enum = (struct enum_info *) malloc (sizeof (enum_info));
bzero (l_enum, sizeof(enum_info));
memcpy (l_enum->enum_name, enum_name, strlen(enum_name));
memcpy (l_enum->enum_value, enum_num, strlen(enum_num));

Thanks
0
Comment
Question by:bcarder
  • 4
  • 3
  • 2
  • +1
10 Comments
 
LVL 4

Expert Comment

by:captainkirk
Comment Utility
1) I assume bzero() presets a piece of memory to zero

2) could you post your list handling code (insert, delete, etc...) and how you are reading and writing the list?
0
 

Accepted Solution

by:
ber earned 100 total points
Comment Utility
You may have a memory overflow, because you are not allocating
enough memory to "l_enum".

Specifically, you allocate "sizeof (enum_info)" bytes of memory to
"l_enum" while you should be allocating "sizeof(struct enum_info)".

You don't show the definition of "enum_info" but if it is smaller
than "struct enum_info" this will cause an overflow past the size of the
allocated buffer [and into the next allocated buffer.]
0
 
LVL 1

Expert Comment

by:BSoeters
Comment Utility
All the code shown seems okay. Note however, that memcpy is not able to deal with overlapping memory regions. However, memmove is.

By the way, you could've used strcpy() just as easily or even easier here:

strcpy (l_enum->enum_name, enum_name);

You would get rid of the strlen() call alltogether.

Which brings me to another detail: doing a memcpy() over strlen() bytes does work in this case, but are you aware that you're not copying the terminating '\0' character of the original strings ? Therefore, you would have to add 1 to the result of strlen(), as strlen() counts the characters in the string, excluding the terminating '\0'.

By calling bzero(), you effectively circumvented that problem.

Have you ever noticed the function memset() by the way ? It could be a runtime-library replacement of your bzero() function.


Further, the problems you're writing about are probably in your list maintenance code, as captainkirk <G> suggested, please post it...
0
 

Author Comment

by:bcarder
Comment Utility
void store_enum_list(struct enum_info *i, struct enum_info **start,
                     struct enum_info **last)
{
   struct enum_info *old, *p;

   if (!*last)
   {
      i->next = NULL;
      i->prior = NULL;
      *last = i;
      *start = i;
      return;
   }

   p = *start;

   old = NULL;
   while (p)
   {
      if (strcmp(p->enum_name, i->enum_name)<0)
      {
         old = p;
         p = p->next;
      }
      else
      {
         if (p->prior)
         {
            p->prior->next = i;
            i->next = p;
            i->prior = p->prior;
            p->prior = i;
            return;
         }
         i->next = p;
         i->prior = NULL;
         p->prior = i;
         *start = i;
         return;
      }
   }
   old->next = i;
   i->next = NULL;
   i->prior = old;
   *last = i;
}


struct enum_info *defaultval_2_enum(char *default_value)
{
   struct enum_info *l_enum2;
   l_enum2 = estart;

   while (l_enum2)
   {
      if (!strncmp (default_value, l_enum2->enum_value, strlen(default_value)))
      {
         return l_enum2;
      }
      l_enum2 = l_enum2->next;
   }
   return NULL;
}
0
 
LVL 1

Expert Comment

by:BSoeters
Comment Utility
--snip--

   old = NULL;
   while (p)
   {
      if (strcmp(p->enum_name, i->enum_name)<0)
      {
         old = p;
         p = p->next;
      }
      else
      {
         if (p->prior)
         {
            p->prior->next = i;
            i->next = p;
            i->prior = p->prior;
            p->prior = i;
            return;
         }
         i->next = p;
         i->prior = NULL;
         p->prior = i;
         *start = i;
         return;
      }
   }
   old->next = i;   // why ? old == NULL
   i->next = NULL;
   i->prior = old;  // why ? old == NULL
   *last = i;       // why ? old == NULL
}

--snip--

should be something like:

*last->next = i;
i->next = NULL;
i->prior = *last;
*last = i;


and about this:

struct enum_info *defaultval_2_enum(char *default_value)
{
   struct enum_info *l_enum2;
   l_enum2 = estart;  // what is estart ?

   while (l_enum2)
   {
      if (!strncmp (default_value, l_enum2->enum_value, strlen(default_value)))
      {
         return l_enum2;
      }
      l_enum2 = l_enum2->next;
   }
   return NULL;
}


the second part seems to be valid though, if estart is defined...
0
How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

 
LVL 1

Expert Comment

by:BSoeters
Comment Utility
woops, old is not NULL, sorry about that, my mistake

but old does not point to the last entry anyway, it points to the entry before the last, if there is one, otherwise, it would most certainly be NULL.

that seems to be the problem with your code. inserting a record just before the last one, instead of as the last one

0
 

Author Comment

by:bcarder
Comment Utility
Answer accepted
0
 

Author Comment

by:bcarder
Comment Utility
Thanks for your help.
0
 
LVL 1

Expert Comment

by:BSoeters
Comment Utility
Ehr, I thought I had provided the best answer, but instead, another gets the points ?

What's wrong with my answer bcarder ?

0
 

Expert Comment

by:ber
Comment Utility
The points were erroneously given to me.
0

Featured Post

How your wiki can always stay up-to-date

Quip doubles as a “living” wiki and a project management tool that evolves with your organization. As you finish projects in Quip, the work remains, easily accessible to all team members, new and old.
- Increase transparency
- Onboard new hires faster
- Access from mobile/offline

Join & Write a Comment

Have you thought about creating an iPhone application (app), but didn't even know where to get started? Here's how: ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Important pre-programming comments: I’ve never tri…
Preface I don't like visual development tools that are supposed to write a program for me. Even if it is Xcode and I can use Interface Builder. Yes, it is a perfect tool and has helped me a lot, mainly, in the beginning, when my programs were small…
The goal of this video is to provide viewers with basic examples to understand opening and reading files in the C programming language.
The viewer will learn how to use and create keystrokes in Netbeans IDE 8.0 for Windows.

771 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

7 Experts available now in Live!

Get 1:1 Help Now