On Sat, 15 Feb 2025 23:32:37 -0500 David Malcolm <dmalc...@redhat.com> wrote:
> > + free(copier); > > There?s a manual free of "copier" here, but there?s are various error- > handling early returns paths that will leak. Maybe just use a > std::string? > > Similarly with ?path?; I think this is always leaked. Maybe > std::string here too. > > Have you tried running the compiler under valgrind? Configure with > ?enable-valgrind-annotations and pass -wrap per=valgrind to the > driver. It's no accident, comrade. ;-) My design criterion: the parser's memory requirements are linear with the input. As grows the COBOL text, so grows the memory consumption. Any more would be wrong; any less is pointless. The parser makes a single pass over the input. It can't "leak" except in a loop. In general it therefore never calls free(3). Only when a string is being built up of consecutive allocations in a loop do we take any care with free. Before anyone suggests that's wasteful of memory, let's remind ourselves of the compiler's design. The input text is transformed into a GENERIC tree. The compiled program *must* fit in memory. Much of that input needs to be retained as debug strings and supplied values. We have tested gcobol on large COBOL inputs, hundreds of thousands of lines. gcobol compiles those programs, as is, without freeing memory, on virtual machines that can barely link cc1. In the instant case, when open_file() fails, the compiler will soon terminate for lack of input. Code generation will not be engaged. The lost strings are literally no concern at all. (I can see why that might not be obvious on first reading, and I hope I don't sound harsh or dismissive.) As for std::string, it would only complicate the front end. Most strings in the parser are either fed back in to some C API or become parts of GENERIC nodes. I admit having been tempted by std::stringstream for concatenation but in the end asprintf did most of what was needed. > Have you tried running the compiler under valgrind? Configure with > ?enable-valgrind-annotations and pass -wrap per=valgrind to the driver. We have not tried that incantation, no. We used valgrind for corruption problems, both times. ;-) We measured performance and memory use with the excellent Linux perf tool. That is how we found the astonishing problems with std::regex (and also with how we were using it). I hope you see that we're taking care, but not too much care, with memory. If there is a loop that is missing a free, we'll fix it, but I bet it won't matter, because these are just string fragments for filenames and such. To dutifully call free for every allocated scrap of parsed input would be counterproductive: error-prone, and little if anything saved. --jkl
