• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 227
  • Last Modified:

Linked List

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
bcarder
Asked:
bcarder
  • 4
  • 3
  • 2
  • +1
1 Solution
 
captainkirkCommented:
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
 
berCommented:
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
 
BSoetersCommented:
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
Independent Software Vendors: We Want Your Opinion

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

 
bcarderAuthor Commented:
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
 
BSoetersCommented:
--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
 
BSoetersCommented:
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
 
bcarderAuthor Commented:
Answer accepted
0
 
bcarderAuthor Commented:
Thanks for your help.
0
 
BSoetersCommented:
Ehr, I thought I had provided the best answer, but instead, another gets the points ?

What's wrong with my answer bcarder ?

0
 
berCommented:
The points were erroneously given to me.
0

Featured Post

VIDEO: THE CONCERTO CLOUD FOR HEALTHCARE

Modern healthcare requires a modern cloud. View this brief video to understand how the Concerto Cloud for Healthcare can help your organization.

  • 4
  • 3
  • 2
  • +1
Tackle projects and never again get stuck behind a technical roadblock.
Join Now