Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > comp.lang.c > #162479
| From | Lew Pitcher <lew.pitcher@digitalfreehold.ca> |
|---|---|
| Newsgroups | comp.lang.c |
| Subject | Re: Freeing a dynamically allocated array of structs within a struct in C |
| Date | 2021-08-29 18:18 +0000 |
| Organization | A noiseless patient Spider |
| Message-ID | <sggj16$1vg$3@dont-email.me> (permalink) |
| References | <1348a974-54cd-47cf-bf40-7034feaa1f78n@googlegroups.com> |
On Sun, 29 Aug 2021 08:34:56 -0700, Bithov Vinu Student wrote:
> Hi,
>
> I have the following definition:
>
> ```
> typedef struct {
> char* entry_key;
> char* entry_value;
> } deck_entry;
> ```
>
> And the following function to parse a string into a deck_entry:
>
> ```
> deck_entry* parse_deck_entry(char* to_parse) {
> char* string_to_parse = strdup(to_parse);
> char* a = strsep(&string_to_parse, ": ");
> char* b = strsep(&string_to_parse, "\n");
> return make_deck_entry(a, b);
> }
> ```
>
> With the test code as follows:
>
> ```
> int main() {
> deck_entry* new_deck = parse_deck_entry("Age: 23\n");
> free_deck_entry(new_deck);
> return 0;
> }
> ```
>
> The program works without segfaulting or anything else, in fact, using a diagnostic function I wrote (print_deck_entry) all the information has been parsed properly with the write formatting, etc. However, running through Valgrind shows:
>
> ```
> ==8494== Memcheck, a memory error detector
> ==8494== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
> ==8494== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
> ==8494== Command: ./box
> ==8494==
> ==8494==
> ==8494== HEAP SUMMARY:
> ==8494== in use at exit: 25 bytes in 1 blocks
> ==8494== total heap usage: 4 allocs, 3 frees, 65 bytes allocated
> ==8494==
> ==8494== LEAK SUMMARY:
> ==8494== definitely lost: 25 bytes in 1 blocks
> ==8494== indirectly lost: 0 bytes in 0 blocks
> ==8494== possibly lost: 0 bytes in 0 blocks
> ==8494== still reachable: 0 bytes in 0 blocks
> ==8494== suppressed: 0 bytes in 0 blocks
> ==8494== Rerun with --leak-check=full to see details of leaked memory
> ==8494==
> ==8494== For lists of detected and suppressed errors, rerun with: -s
> ==8494== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
> ```
>
> Modifying parse_deck_entry to be the following:
>
> ```
> deck_entry* parse_deck_entry(char* to_parse) {
> char* string_to_parse = strdup(to_parse);
> char* a = strsep(&string_to_parse, ": ");
> char* b = strsep(&string_to_parse, "\n");
> deck_entry* tmp = make_deck_entry(a, b);
> free(a);
> free(b);
> free(string_to_parse);
> return tmp;
> }
> ```
>
> ...will still compile but prints the following when run:
>
> ```
> free(): invalid pointer
> Aborted (core dumped)
> ```
>
> I can't imagine anything else that could be causing a memory leak, considering parse_deck_entry() is the only function I've written that is being called in the test code. What have I done wrong and what could I do to improve? Stability and memory-safety are pretty huge for what I'm writing, even though I am writing in C (rather than Rust, etc.)
Let's ignore your revised parse_deck_entry() function for now, and concentrate
on the original parse_deck_entry() that leaked memory.
> deck_entry* parse_deck_entry(char* to_parse) {
> char* string_to_parse = strdup(to_parse);
> char* a = strsep(&string_to_parse, ": ");
> char* b = strsep(&string_to_parse, "\n");
> return make_deck_entry(a, b);
> }
Please bear with me while I reformat your function for clarity:
deck_entry *parse_deck_entry(char *to_parse)
{
char *string_to_parse,
*a,
*b;
string_to_parse = strdup(to_parse);
a = strsep(&string_to_parse,": ");
b = strsep(&string_to_parse,"\n");
return make_deck_entry(a,b);
}
Note that you (implicitly) malloc() memory with the strdup()
call that assigns to string_to_parse. Thus, string_to_parse
contains a pointer that you should, at some point, pass unaltered
to free(), to release the allocated memory.
Now, strsep()
"finds the first token in the [input] string that is delimited
by one of the bytes in the [delimiter] string. This token is
terminated by overwriting the delimiter with a null byte ('\0'),
and [the input string pointer] is updated to point past the token."
This means that each call to strsep() not only updates the /contents/
of the array pointed to by string_to_parse, it also updates the
string_to_parse pointer itself.
So, by using strsep(), you've lost the pointer value that you must give
free() to return the malloc()'ed memory (from strdup()) to the memory
pool.
As string_to_parse is an object local to the parse_deck_entry() function,
and not passed to other functions, it is clear that it is /not/ passed
to free() at any time, and thus, you leak memory.
The fix would be to, within parse_deck_entry(), free() the /original/
value of string_to_parse, after you've used the substrings you coaxed
out with strsep(). But, of course, you've destroyed the original value
of string_to_parse, /and/ (even if you hadn't destroyed it) you would
need to free() that value /after/ the call to make_deck_entry() (in
order to not deallocate the memory pointed to by a and b) but /before/
the return from parse_deck_entry().
So what do you do? You restructure your parse_deck_entry() such that
a) you cache the pointer from strdup() in a variable that you do not
alter,
b) you cache the pointer from make_deck_entry()
c) you free() the cached strdup() result pointer, after the call to
make_deck_entry(), and
d) you return the cached make_deck_entry() pointer
Something like:
deck_entry *parse_deck_entry(char *to_parse)
{
char *temp, /* cache for unaltered strdup() return value */
*string_to_parse, /* strdup() return value, will be altered */
*a,
*b;
deck_entry *result; /* cache for make_deck_entry() return value */
temp = string_to_parse = strdup(to_parse);
a = strsep(&string_to_parse,": "); /* alters string_to_parse */
b = strsep(&string_to_parse,"\n"); /* alters string_to_parse */
result = make_deck_entry(a,b); /* uses storage allocated by strdup() */
free(temp); /* uses unaltered strdup() pointer */
return result;
}
HTH
Addendum: Please notice how I (re)formatted the various object declarations.
With your (apparently) preferred style
char* string_to_parse
you can misread multiple declarations. For instance,
char* string_to_parse, a b;
/does not/ declare
a
to be a pointer to char, even though a naive reading of the
declaration specifiers would lead you to believe so.
However,
char *string_to_parse, a;
has less chance to be misread in that manner.
HTH
--
Lew Pitcher
"In Skills, We Trust"
Back to comp.lang.c | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Freeing a dynamically allocated array of structs within a struct in C Bithov Vinu Student <vinub@calday.co.uk> - 2021-08-29 08:34 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-29 15:43 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-29 16:12 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Tim Rentsch <tr.17687@z991.linuxsc.com> - 2021-09-06 05:46 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Ben Bacarisse <ben.usenet@bsb.me.uk> - 2021-08-29 17:17 +0100
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-29 20:01 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-29 18:21 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-29 20:34 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-30 09:01 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Robert Latest <boblatest@yahoo.com> - 2021-08-31 05:44 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Kaz Kylheku <563-365-8930@kylheku.com> - 2021-08-31 07:50 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-31 10:59 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-30 09:45 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-30 10:15 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Kaz Kylheku <563-365-8930@kylheku.com> - 2021-08-29 18:14 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-29 18:18 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Barry Schwarz <schwarzb@delq.com> - 2021-08-29 11:44 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-29 19:04 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Barry Schwarz <schwarzb@delq.com> - 2021-08-29 14:15 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C scott@slp53.sl.home (Scott Lurndal) - 2021-08-29 21:19 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Barry Schwarz <schwarzb@delq.com> - 2021-08-29 18:14 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-30 18:27 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-30 16:17 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C James Kuyper <jameskuyper@alumni.caltech.edu> - 2021-08-30 14:14 -0400
Re: Freeing a dynamically allocated array of structs within a struct in C Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2021-08-30 11:39 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C James Kuyper <jameskuyper@alumni.caltech.edu> - 2021-08-31 10:08 -0400
Re: Freeing a dynamically allocated array of structs within a struct in C Kaz Kylheku <563-365-8930@kylheku.com> - 2021-09-01 04:40 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2021-09-01 00:57 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Lew Pitcher <lew.pitcher@digitalfreehold.ca> - 2021-08-30 19:16 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Manfred <noname@add.invalid> - 2021-08-30 21:49 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C "Chris M. Thomasson" <chris.m.thomasson.1@gmail.com> - 2021-08-30 12:49 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C scott@slp53.sl.home (Scott Lurndal) - 2021-08-30 22:31 +0000
Re: Freeing a dynamically allocated array of structs within a struct in C Tim Rentsch <tr.17687@z991.linuxsc.com> - 2021-09-30 07:04 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Keith Thompson <Keith.S.Thompson+u@gmail.com> - 2021-08-29 15:38 -0700
Re: Freeing a dynamically allocated array of structs within a struct in C Manfred <noname@add.invalid> - 2021-08-30 14:21 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-30 18:29 +0200
Re: Freeing a dynamically allocated array of structs within a struct in C Bart <bc@freeuk.com> - 2021-08-30 18:30 +0100
Re: Freeing a dynamically allocated array of structs within a struct in C Bonita Montero <Bonita.Montero@gmail.com> - 2021-08-31 04:22 +0200
csiph-web