There are a number of issues that will bite you in your code. While you are free to allocate storage for name
, that will complicate things quite a bit if you also assign string literals to the first 2 elements of brand
. You then will have mixed literals and allocated blocks of memory that you will have to keep track of in order to know what needs to be freed later on.
Additionally, when using scanf
to read multiple values, you are better off reading an entire line into a buffer with fgets
and then calling sscanf
to parse the values. There are a number of reasons why decoupling your read from your parse makes sense.
Also, if name is short, then why not use an array and eliminate the dynamic allocation altogether? For purposes of your example, where you are assigning literals to the first two, that also solves your problem of which will need to be free
d and which will segfault if you try to free them.
I've included more in the comments below. Look things over and let me know if you have further questions:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
enum { ITEMS = 5, MAXN = 32, MAXC = 256 };
typedef struct {
char name[MAXN];
int year, amount;
} inventory;
int main (void) {
/* you do not want to mix string literals below for 'name' with allocated
* blocks of memory also used for 'name' because the logic to free the allocated
* blocks would be unnecessarily complicated. (e.g., some 'name's allocated,
* some holding literals) Keep your code consistent. If the length of name is
* reasonably short, just use an array.
*/
inventory brand[ITEMS] = { {"chevy", 2014, 12}, {"acura", 2016, 22} };
int num = 2;
while (num < ITEMS) {
char buf[MAXC] = "";
/* no need for ptr, it's up to you, brand[num] with dot notation is fine */
inventory *ptr = &brand[num];
printf ("Enter a brand of automobile, year, and count "
"of the vehicle with a space in between:\n");
/* if name is a pointer, you must allocate space before calling scanf
* (unless using the 'm' modifier ('a' for older scanf and windoze) and
* you then provide a pointer-to-pointer-to-char) Using an array avoids
* that problem wit name.
*/
if (!fgets (buf, MAXC, stdin) ||
sscanf (buf, " %s %d %d", ptr->name, &ptr->year, &ptr->amount) != 3) {
fprintf (stderr, "error: invalid input.\n");
continue;
}
/* allocating 1-char for ptr is somewhat pointless here */
// if ( !(ptr = malloc (sizeof(char)) == NULL)) {
/* you would allocate the sizeof one struct, or just s, however using
* an array for name makes allocation unnecessary.
*/
// if (!(ptr = malloc (sizeof s))) {
// fputs ("Out of memory", stderr); /* fputs adds a newline */
// exit (EXIT_FAILURE);
// }
/* memcpy -- WTF?? (there is no memcpy here, what is weight, calories?) */
// printf("%s\t%d\t%d\n", ptr->name, ptr->weight, ptr->calories);
/* you probably want */
// free(ptr); (no allocation, no free required
num++;
}
/* now lets see if it worked */
for (int i = 0; i < num; i++)
printf ("\n name : %s\n year : %d\n amount : %d\n",
brand[i].name, brand[i].year, brand[i].amount);
return 0;
}
(lastly, leave MixedCase variable names for C++. C style generally prefers all lower-case, reserving upper-case for constants and macros. While style is completely up to you, an odd style telegraphs a lot about your code before any of the details are reached)
Example Use/Output
$ ./bin/inventory2
Enter a brand of automobile, year, and count of the vehicle with a space in between:
ford 1968 7
Enter a brand of automobile, year, and count of the vehicle with a space in between:
foo bar
error: invalid input.
Enter a brand of automobile, year, and count of the vehicle with a space in between:
mazda 2007 9
Enter a brand of automobile, year, and count of the vehicle with a space in between:
toyota 2012 4
name : chevy
year : 2014
amount : 12
name : acura
year : 2016
amount : 22
name : ford
year : 1968
amount : 7
name : mazda
year : 2007
amount : 9
name : toyota
year : 2012
amount : 4