Bug 187174 - [Linux] Fix memory leak in WTF::forEachLine()
Summary: [Linux] Fix memory leak in WTF::forEachLine()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alicia Boya García
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-06-29 04:00 PDT by Alicia Boya García
Modified: 2018-07-02 04:41 PDT (History)
11 users (show)

See Also:


Attachments
Patch (1.20 KB, patch)
2018-06-29 04:15 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff
Patch (1.55 KB, patch)
2018-07-02 03:26 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff
Patch for landing (1.55 KB, patch)
2018-07-02 04:02 PDT, Alicia Boya García
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2018-06-29 04:00:33 PDT
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);
}
Comment 1 Alicia Boya García 2018-06-29 04:15:30 PDT
Created attachment 343904 [details]
Patch
Comment 2 Adrian Perez 2018-06-29 04:19:10 PDT
Comment on attachment 343904 [details]
Patch

Informal r+ from me, good catch! :-)
Comment 3 Yusuke Suzuki 2018-06-29 05:17:02 PDT
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.
Comment 4 Alicia Boya García 2018-06-29 05:26:06 PDT
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.
Comment 5 Zan Dobersek 2018-07-01 23:08:43 PDT
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.
Comment 6 Alicia Boya García 2018-07-02 03:26:31 PDT
Created attachment 344089 [details]
Patch
Comment 7 WebKit Commit Bot 2018-07-02 03:47:11 PDT
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
Comment 8 Alicia Boya García 2018-07-02 04:02:01 PDT
Created attachment 344092 [details]
Patch for landing
Comment 9 WebKit Commit Bot 2018-07-02 04:41:36 PDT
Comment on attachment 344092 [details]
Patch for landing

Clearing flags on attachment: 344092

Committed r233420: <https://trac.webkit.org/changeset/233420>
Comment 10 WebKit Commit Bot 2018-07-02 04:41:38 PDT
All reviewed patches have been landed.  Closing bug.