Currently, I try to solve an excercise in an exam for preparation. After researching here, I changed the structure definition of
char nr[10];
char name[25];
to
char *nr;
char *name;
to pass a string (a pointer to a string) into the structure (see the whole code below).
When I execute the code with Netbeans 8.2 and Cygwin GCC 7.4.0 I got a runtime error. Sometimes the code runs endlessly w/o termination, sometimes I got a segmentation error. After debugging, the error occurs once the compiler tries to execute
new->item->nr = orderno;
What I realized by another excercise w/ bubble sort where you've a swap function. I'm used to swap the referenced values like this:
void swap(int *px, int *py) {
int temp = *px;
*px = *py;
*py = temp;
}
What I'd like to do is swapping by using the addresses like this:
void swap(int *px, int *py) {
int *temp = px;
px = py;
py = temp;
}
Netbeans shows the same behavior by running endlessly w/o termination either the swap function or the code I'm creating ths topic for. Does anybody have an idea what I'm doing wrong?
#include <stdio.h>
#include <stdlib.h>
typedef struct litem_t {
char *nr; // was char nr[10];
char *name; // was char name[25];
float price;
int cnt;
float sum;
} litem;
typedef struct llist_t {
struct litem_t *item;
struct llist_t *next;
} llist;
llist* addItem(llist *alist, char *orderno, char *name, float price, int count, float sum);
void dumpList(llist *alist);
void L_3_a_KL_17(int argc, char **argv) {
llist *alist;
alist = addItem(NULL, "AT 1001 C", "Pear jPhone Quad 64GB", 499.00, 1, 499.00);
alist = addItem(alist, "ZT 1012 D", "Pear jPhone Octa 128GB", 799.00, 1, 799.00);
dumpList(alist);
}
llist* addItem(llist *alist, char *orderno, char *name, float price, int count, float sum) {
llist *new = (llist *)malloc(sizeof(llist));
if(new == NULL)
return NULL;
new->item->nr = orderno; // runtime error here
new->item->name = name;
new->item->price = price;
new->item->cnt = count;
new->item->sum = sum;
new->next = alist;
return new;
}
void dumpList(llist *alist) {
llist *temp;
int j = 1;
while(alist != NULL) {
printf("\n%d. Article:\nNumber:\t%s\nName:\t%s\nPrice:\t%.2f\nCnt:\t%d\nSum:\t%.2f",
j, alist->item->nr, alist->item->name, alist->item->price, alist->item->cnt,
alist->item->summe);
temp = alist;
alist = alist->next;
free(temp);
j++;
}
}
Basically, the code should read articles, store them sequentially in a concatenated list and print them on the screen afterwards. The sequence of the articles doesn't matter.
Thanks for any help.
KR
In your list source there are a few mistakes
With
ptr->item->nr = orderno;
you copy the pointer to a string to the pointer in the struct. This works only if the string is already stored on the heap and you pass the pointer as parameter in the function call.
In your example you copy the string address only. If that string is not longer available if the function is returned, the pointer does not point to the expected string. You should copy the strings to buffers as you have defined them in your first version or make sure that the strings exists as long as the pointer inside the data struct has its value.
The second problem is that you allocate memory for the list entry but not for the item it-self. That causes the error your mentioned. The list entry contains only a pointer to the item! So you have to allocate memory for the item as well. If this fails you have to error handle the problem that you have successfully created a list element but not the data item.
For answering the question in your comment please let me go into some details
You define a struct type for data
typedef struct litem_t {
char nr[10]; // was char nr[10];
char name[25]; // was char name[25];
float price;
int cnt;
float sum;
} litem;
and a struct with pointers to chain the items:
typedef struct llist_t {
struct litem_t *item;
struct llist_t *next;
} llist;
Please look at this picture. You can see that you need to create for both structs an instance for each new data
Chained Instances data instances
of type llist of type litem
Size of each Size of each element
instance is 8 byte is 47 byte
[alist]
|
|
'--> +--------+ last created objects
| item --|---------------> +-----------+
| next --|---. | nr[10] | 10 byte
+--------+ | | name[25] | 25 byte
| | price | 4 byte
.-----------------' | cnt | 4 byte
| | sum | 4 byte
| +-----------+
'--> +--------+
| item --|---------------> +-----------+
| next --|---. | nr[10] |
+--------+ | | name[25] |
| | price |
.-----------------' | cnt |
| | sum |
| +-----------+
'--> +--------+
| item --|---------------> +-----------+
| next = NULL | nr[10] |
+--------+ | name[25] |
| price |
| cnt |
| sum |
+-----------+
First created objects
The picture show that you first need to allocate memory for the llist item. Than the pointer "item" exists and you can allocate memory for the data item. Both items have to be freed separately, for example like this.
llist * next;
if (alist)
{
do
{
next = alist->next; // temp. copy because pointer will be deleted
if (alist->item) free(alist->item); // free memory for litem
free (alist); // free memory for llist
alist = next; // set alist to next item
} while (alist != NULL) /7 very first entry has no "next"
}
Better would be create a function which frees a single item and correct the "next" pointer of the previous item. With this you could remove an item of the list. Such is IMHO one reason for using a chained list.
Code (without free)
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct litem_t {
char nr[10]; // was char nr[10];
char name[25]; // was char name[25];
float price;
int cnt;
float sum;
} litem;
typedef struct llist_t {
struct litem_t *item;
struct llist_t *next;
} llist;
llist* addItem(llist *alist, char const *orderno, char const *name, float price, int count, float sum);
void dumpList(llist *alist);
void L_3_a_KL_17(int argc, char **argv) {
llist *alist;
alist = addItem(NULL, "AT 1001 C", "Pear jPhone Quad 64GB", 499.00, 1, 499.00);
alist = addItem(alist, "ZT 1012 D", "Pear jPhone Octa 128GB", 799.00, 1, 799.00);
dumpList(alist);
}
llist* addItem(llist *alist, char const *orderno, char const *name, float price, int count, float sum) {
// Alloccate space for new list entry
llist *newlistentry_ptr = (llist *)malloc(sizeof(llist));
if(newlistentry_ptr == NULL)
return NULL;
// Alloc space for new data item
newlistentry_ptr->item = (litem *)malloc(sizeof(litem));
if(newlistentry_ptr->item == NULL) {
// add here some error handling
// caller will not expect that item is NULL
// maybe free newlistentry_ptr for error detection
newlistentry_ptr->next = alist;
return newlistentry_ptr;
}
// Copy strings into a buffer where they are retained
strncpy (newlistentry_ptr->item->nr, orderno, sizeof(newlistentry_ptr->item->nr));
strncpy (newlistentry_ptr->item->name, name, sizeof(newlistentry_ptr->item->name));
newlistentry_ptr->item->price = price;
newlistentry_ptr->item->cnt = count;
newlistentry_ptr->item->sum = sum;
newlistentry_ptr->next = alist;
return newlistentry_ptr;
}
void dumpList(llist *alist) {
llist *temp;
int j = 1;
while(alist != NULL) {
printf("\n%d. Article:\nNumber:\t%s\nName:\t%s\nPrice:\t%.2f\nCnt:\t%d\nSum:\t%.2f",
j, alist->item->nr, alist->item->name, alist->item->price, alist->item->cnt,
alist->item->sum);
temp = alist;
alist = alist->next;
free(temp);
j++;
}
}