Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###Memory leak, plus confusion###

Memory leak, plus confusion

In init_node(), you have this code:

 n->data = malloc(type_size);
 if (n != NULL && n->data != NULL) {
 n->data = element;

Immediately, I see a memory leak because you allocate some memory and assign it to a pointer, and then immediately reassign the pointer to something else.

I'm confused about how you intended for your list to work. Either the list should make a copy of the input element, or it should retain a pointer to input element. It looks like you tried to do a little bit of both.

To do it correctly, either:

  1. Do the malloc like you did in the first line above, but instead of the third line, use memcpy to copy the contents of the input element.

  2. Eliminate the first line with the malloc, and keep the assignment from the third line.

###Memory leak, plus confusion###

In init_node(), you have this code:

 n->data = malloc(type_size);
 if (n != NULL && n->data != NULL) {
 n->data = element;

Immediately, I see a memory leak because you allocate some memory and assign it to a pointer, and then immediately reassign the pointer to something else.

I'm confused about how you intended for your list to work. Either the list should make a copy of the input element, or it should retain a pointer to input element. It looks like you tried to do a little bit of both.

To do it correctly, either:

  1. Do the malloc like you did in the first line above, but instead of the third line, use memcpy to copy the contents of the input element.

  2. Eliminate the first line with the malloc, and keep the assignment from the third line.

Memory leak, plus confusion

In init_node(), you have this code:

 n->data = malloc(type_size);
 if (n != NULL && n->data != NULL) {
 n->data = element;

Immediately, I see a memory leak because you allocate some memory and assign it to a pointer, and then immediately reassign the pointer to something else.

I'm confused about how you intended for your list to work. Either the list should make a copy of the input element, or it should retain a pointer to input element. It looks like you tried to do a little bit of both.

To do it correctly, either:

  1. Do the malloc like you did in the first line above, but instead of the third line, use memcpy to copy the contents of the input element.

  2. Eliminate the first line with the malloc, and keep the assignment from the third line.

Source Link
JS1
  • 28.9k
  • 3
  • 41
  • 83

###Memory leak, plus confusion###

In init_node(), you have this code:

 n->data = malloc(type_size);
 if (n != NULL && n->data != NULL) {
 n->data = element;

Immediately, I see a memory leak because you allocate some memory and assign it to a pointer, and then immediately reassign the pointer to something else.

I'm confused about how you intended for your list to work. Either the list should make a copy of the input element, or it should retain a pointer to input element. It looks like you tried to do a little bit of both.

To do it correctly, either:

  1. Do the malloc like you did in the first line above, but instead of the third line, use memcpy to copy the contents of the input element.

  2. Eliminate the first line with the malloc, and keep the assignment from the third line.

lang-c

AltStyle によって変換されたページ (->オリジナル) /