[Solved] Ex32: testcase to break the list, does not break: why?

Doing exercise 32 I noticed that the list frees the elements of the list, even though it has no idea whether the element was created with malloc/calloc. The testcases for the list use string elements that were defined in file scope, not created with malloc. So I added a testcase with a printf of each element after clear and destroy of the list. To my surprise it just runs and prints the 3 elements, even though the list function for clear frees it.

What am I overlooking here? Why does it run?

P.S. The code for the test is:

char *test_access()
{
    printf("test1 = %s\t", test1);
    printf("test2 = %s\t", test2);
    printf("test3 = %s\n", test3);

   return NULL;
}

The output is:

RUNNING: ./tests/list_tests
test1 = test1 data	test2 = test2 data	test3 = test3 data
ALL TESTS PASSED
Tests run: 7
tests/list_tests PASS

I would have expected access to the fields to at least segfault, because the list does a free for each element.

EDIT: changed the title so it covers the contents better

I don’t think this is all of the code is it? I don’t see where these test variables are being defined so I can’t tell why they should break. Looks like there actually defines just fine they seem to be printing out some data.

The test variables are defined as follows:

  static List *list = NULL;
   
  char *test1 = "test1 data";
  char *test2 = "test2 data";
  char *test3 = "test3 data";

So that’s not the issue. The issue is that after the addition of these test items to the list, (a static field) the function List_clear or List_clear_destroy frees all elements present in the list. The code is:

 void List_clear(List *list)
 {
      LIST_FOREACH(list, first, next, cur)
      {
          free(cur->value);
      }
  }

Now the code cur->value frees one of the items test1, test2 or test3. My concern is that these items were never allocated with malloc or calloc. For that reason I expected the free(cur->value) to give major trouble. Actuallly, I expected it to dump when being freed, but it didn’t. So I made a testcase to make it dump by referencing the items that were previously freed in the list.

Well, needless to say I am just a little confused how this works.

Ah, that is wildly different from the problem you described above. Yes, if you’re doing a clear on the list, then when you add elements to the list you need to duplicate them so they are allocated by malloc. In this case strdup() would work, or using the bstring library instead.

Now, as for why it doesn’t crash: That’s C. C just lets things that should crash in any other reasonable language keep on going. I bet if you ran this under gdb or valgrind you’d probably see these as errors. Valgrind better show these as errors, but gdb might not.

Actually, no, it is the same question. I just neglected to show all the code (it was part of exercise 32).

In exercise 32. we create a list where we free but do not allocate. As far as I can see we can’t because we don’t have the information: what we get is a void *value so we have no idea how much memory to allocate. Also, we couldn’t use strdup() because we don’t know it is a char *, it could be an integer pointer, a float pointer or even a struct pointer. We have no idea.

When I studied the book, I really thought the testcase as defined should break the list, because of the free. If I understand you correctly, it should indeed, but doesn’t due to the character of implementations of C. Should the list in exercise 32 now be altered? Should we remove the free from exercise 32? If so, what happens to the pointers? If they were malloced, they need to be free-ed. Otherwise clear doesn’t have to free them. Seems like a predicament to me.

If we make the list type specific, then we could strcpy the value, but the tests would break, because they test for pointer equality after adding to the list.

If we make the list generic, then we need to know the amount of memory for the allocate in order to copy the memory. Also, we cannot use strdup because it might not be a char *value. I suspect memcpy is the candidate. In this case the test as currently set up would break, because it tests for pointer equality, which is no longer true due to the copy.

Sorry Zed, I am not trying to be a nuisance, these are just the thoughts my “critical system” came with when reviewing the list and test code. If this goes beyond the test of the book and the exercises, then we could just let it be.

Not a problem, this could be a bug actually. Here’s the code to the tests for List:

And the code for the list itself:

If you look at List_destroy, it doesn’t call free on the value. That’s List_clear. Now, looking at the test case, I don’t call List_clear() or List_clear_destroy() on any of those lists I make except an empty one.

You have this though:

char *test_access()
{
    printf("test1 = %s\t", test1);
    printf("test2 = %s\t", test2);
    printf("test3 = %s\n", test3);

   return NULL;
}

And you say "So I added a testcase with a printf of each element after clear and destroy of the list. ". Welllllll, don’t do that. I’m not doing it, I’m doing List_destroy(), which does not free these.

Now, whether it causes an error or not totally depends on the platform and chance. That’s what makes C such a terrible language to work with. Other languages would flag this as an error, but C just keeps on going. Oh well.

Hi Zed,

Thank you very much for your answer (and your time). I overlooked the fact that you do not execute the clear. A pretty major oversight with respect to the problem I was trying to formulate. So, I apologize for not being thorough enough. It’s a big lession tho (embarrasment always is), thank you.

The question I am left with is one of strategy. Wrong use could lead to a segmentation fault or worse. It could overwrite memory that has valid other definitions in it. So, what strategy would you recommend? Would you leave out clear altogether? Would you leave clear in and document the fact that the user should only call clear on malloc-ed fields?

It is the query it all boils down to: do we or do not do the free in the library? Or, should we have the user handle both creation and de-allocation?

Yeah, that is always the problem with C since it doesn’t have automated memory management. I generally go with making you deal with deallocation.