Bug 187174 – [Linux] Fix memory leak in WTF::forEachLine()

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Alicia Boya García]

getline() allocates a buffer even when the read fails because EOF. This buffer also needs to be freed.

Also note the buffer is intended to be reused to avoid making an allocation each time a line is read, getline() will realloc() the buffer autmatically if necessary.

See `man getline()` for an example:

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>

int
main(int argc, char *argv[])
{
   FILE *stream;
   char *line = NULL;
   size_t len = 0;
   ssize_t nread;

   if (argc != 2) {
       fprintf(stderr, "Usage: %s <file>\n", argv[0]);
       exit(EXIT_FAILURE);
   }

   stream = fopen(argv[1], "r");
   if (stream == NULL) {
       perror("fopen");
       exit(EXIT_FAILURE);
   }

   while ((nread = getline(&line, &len, stream)) != -1) {
       printf("Retrieved line of length %zu:\n", nread);
       fwrite(line, nread, 1, stdout);
   }

   free(line);
   fclose(stream);
   exit(EXIT_SUCCESS);
}

[Alicia Boya García]

Created attachment 343904 [details]
Patch

[Adrian Perez]

Comment on attachment 343904 [details]
Patch

Informal r+ from me, good catch! :-)

[Yusuke Suzuki]

Comment on attachment 343904 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343904&action=review

> Source/WTF/wtf/linux/MemoryFootprintLinux.cpp:43
>      while (getline(&buffer, &size, file) != -1) {

We should get the size of the line string here (lineLength = getline(...)), since “size” is the length of the allocated buffer.

[Alicia Boya García]

Comment on attachment 343904 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343904&action=review

>> Source/WTF/wtf/linux/MemoryFootprintLinux.cpp:43
>>      while (getline(&buffer, &size, file) != -1) {
> 
> We should get the size of the line string here (lineLength = getline(...)), since “size” is the length of the allocated buffer.

Actually, size is never read in the functor, it's an unused argument.

[Zan Dobersek]

Comment on attachment 343904 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=343904&action=review

>>> Source/WTF/wtf/linux/MemoryFootprintLinux.cpp:43
>>>      while (getline(&buffer, &size, file) != -1) {
>> 
>> We should get the size of the line string here (lineLength = getline(...)), since “size” is the length of the allocated buffer.
> 
> Actually, size is never read in the functor, it's an unused argument.

Please remove it before landing.

[Alicia Boya García]

Created attachment 344089 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 344089 [details]
Patch

Rejecting attachment 344089 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 344089, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Zan Dobersek found in /Volumes/Data/EWS/WebKit/Source/WTF/ChangeLog does not appear to be a valid reviewer according to contributors.json.
/Volumes/Data/EWS/WebKit/Source/WTF/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: https://webkit-queues.webkit.org/results/8410850

[Alicia Boya García]

Created attachment 344092 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 344092 [details]
Patch for landing

Clearing flags on attachment: 344092

Committed r233420: <https://trac.webkit.org/changeset/233420>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.