Click here to Skip to main content
15,885,214 members
Please Sign up or sign in to vote.
0.00/5 (No votes)
See more:
I'll be honest, I've been working on an assignment for
about 10 hours and I cannot for the life of me understand Dynamic Memory
Allocation. This code is supposed to prompt the user for a name, weight, and calorie count of a food. The name is then supposed to be stored in a temporary char buffer. Next the exact amount of space necessary to represent the food name with its null terminator. After that I am supposed to dynamically allocate that exact amount of memory and store the pointer to it in the name member of the food structure. Finally I copy the food's name using the memcpy function. I was also told to use malloc over calloc and realloc. When I run the code I receive Memory access errors, most likely because I don't believe I am allocating properly.
Code:
C++
#define LUNCH_QTY 3
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
   struct Food
   {
      char* name; int weight, calories;
   } lunches[LUNCH_QTY] = {{(char*)"apple", 4, 100}, {(char*)"salad", 2, 80}};

   for (int element = 2; element < LUNCH_QTY; element++)
   {
      char name[100];
      printf("Please enter, space-separated, the name, weight and calories of a food: ");
      scanf("%s %d %d", &name, &lunches[element].weight, &lunches[element].calories);
      if ((lunches[element].name = (char*)malloc(strlen(name))) == NULL)
      {
         fputs("Unable to allocate memory\n", stderr);
         exit(1);
      }
      memcpy(lunches[element].name, &name, strlen(name));
      printf("%s %d %d", lunches[element].name, lunches[element].weight, lunches[element].weight);
      free(name);
      name = NULL;
   }
}
Any help is much appreciated

What I have tried:

I have tried using Malloc on name as well as on the name member of the food structure. I've also tried adding & in about every spot I can think of and removing it as well.
Posted
Updated 25-Aug-20 20:26pm
v2

malloc allocates a block of uninitialized, contiguous memory of the requested size in bytes and passes back a void* pointer since it has no idea what you want that memory for.

So, how do we calculate how many bytes we need then? Well, in this instance we have an array of characters, so we need space for each element (char) and the overall string size. To get the size of the element, we can use sizeof(char). To get the overall array size, we can use strlen(name) + 1.

This is your code:
C++
(char*)malloc(strlen(name)) //If name is "apple", strlen will return 5 (it doesn't include the termination character '\0'). See: http://www.cplusplus.com/reference/cstring/strlen/


You see the issue? If you put a breakpoint at the printf line and look at the lunches[2] structure you'll see the name followed by random garbage because there isn't a null-termination character. This is why we need that +1 on both the malloc and memcpy to allocate space for the string terminator and to copy it, respectively.

Also you're getting lucky that the size of a char is a single byte, otherwise your malloc wouldn't allocate the correct size. Generally speaking, you want to get in the habit of doing it like malloc(sizeof(char) * (strlen(name) + 1)). It will lead to less headaches down the road (a char isn't always 1 byte). The "+1" won't be needed, for example, with an integer array though - malloc(sizeof(int) * arraySize).

This has nothing to do with your memory access problem though. That is caused by
C++
free(name);
name = NULL;

You only free what you malloc. name is stack-allocated. It will be discarded when the stack-frame is popped on function exit. This is also why the line name = NULL; should show an error in your compiler. You can't re-assign name. It isn't a pointer to a char array, it is the char array. Did you mean to free the structure's name but accidentally referenced the local name instead?
 
Share this answer
 
v7
Comments
Ryan Thomsen 25-Aug-20 23:17pm    
Thank you very much for you response, it helped clear up quite a few things actually. I feel like I've followed most of your changes, however I'm struggling to figure out some of the bugs. I'm getting a C6385,C6386, and a 6054 warning. When I run the program it never makes it past entering the input with scanf. Is there something I did wwrong?
 #define LUNCH_QTY 3
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
   struct Food
   {
      char* name; int weight, calories;
   } lunches[LUNCH_QTY] = {{(char*)"apple", 4, 100}, {(char*)"salad", 2, 80}};

   for (int element = 2; element <= (LUNCH_QTY); element++)
   {
      char name[100];
      printf("Please enter, space-separated, the name, weight and calories of a food: ");
      if (scanf("%99s %d %d", name, &lunches[element].weight, &lunches[element].calories) == 1)
      {
         lunches[element].name = (char*)malloc(strlen(name)+1);
         if (lunches[element].name)
         {
            memcpy(lunches[element].name, &name, strlen(name)+1);
         }
         else
         {
            fputs("Unable to allocate memory\n", stderr);
            exit(1);
         }
      }
      else
      {
         fputs("Unable to read input\n", stderr);
         exit(1);
      }

      printf("%s %d %d", lunches[element].name, lunches[element].weight, lunches[element].weight);
      free(lunches[element].name);
      lunches[element].name = NULL;
   }
} 
Jon McKee 25-Aug-20 23:37pm    
That's a lot of changed code.
- Think about that for loop carefully, specifically <=. You're creating 3 lunches (lunches[LUNCH_QTY]), but arrays are zero-indexed, so what total lunch count is lunches[3]?


Nvm, you can use just name. scanf has no issues with just using name as Rick's example works. It's been a minute since I've used good ole C. Your original example with my changes runs fine, and Rick's example below with my notes runs fine, so you've got two examples to work from :)
Jon is right on the nose. Your for loop should be rewritten to look something like this :
C++
struct Food
{
   char* name;
   int weight;
   int calories;
};
struct Food lunches[ LUNCH_QTY ];
struct Food * pfood;
char name[ 100 ] = { 0 };
int memorysize;
for (int element = 0; element < LUNCH_QTY; element++)
{
   pfood = & lunches[ element ];
   printf( "Please enter, space-separated, the name, weight, and calories of a food: ");
   scanf( "%s %d %d", name, & pfood->weight, & pfood->calories );
   memorysize = strlen( name ) + 1;   // include the null
   pfood->name = (char*)malloc( memorysize );
   if( pfood->name == NULL )
   {
      fprintf( stderr, "Unable to allocate memory\n" );
      exit(1);
   }
   memcpy( pfood->name, name, memorysize );
}

for( int element = 0; element < LUNCH_QTY; element++ )
{
   pfood = & lunches[ element ];
   fprintf( stdout, "%d : %s %d %d\n",
       element + 1, pfood->name, pfood->weight, pfood->calories );

   free( pfood->name );
   pfood->name = NULL;
}

I hope you understand it is very bad practice to do what your code is doing. That is, it is mixing static data from initialization and dynamically allocated data in the same structure. The reason that is bad is you can't release static data and this data requires external information to know the difference. You are better off not initializing any data and entering it all so you can release the allocated strings later. I changed your code to do this.

Also, this uses a pointer to access the Food structure because it simplifies the code and makes it easier to understand. Plus, the code can be easier to adapt for other purposes if the pointer is used.
 
Share this answer
 
v3
Comments
Jon McKee 25-Aug-20 23:56pm    
+1: I like that you point out a better strategy for initializing things.

This is C though, so you need struct Food lunches[LUNCH_QTY] and struct Food * pfood. Also in the scanf, it should be &pfood->weight and &pfood->calories. With those changes this compiles and runs fine :)
Rick York 26-Aug-20 1:43am    
Thanks. I haven't dealt with C in a long time and I always forget its differences with C++.
Jon McKee 26-Aug-20 1:56am    
Same. It took me a minute to figure out why my IDE was saying Food wasn't a type, lol.
Rick York 26-Aug-20 20:10pm    
That reminds of why I used to make them all typedefs - so they didn't need the struct.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900