| Summary: | [Linux] Fix memory leak in WTF::forEachLine() | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alicia Boya García <aboya> | ||||||||
| Component: | WebKitGTK | Assignee: | Alicia Boya García <aboya> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | aperez, benjamin, bugs-noreply, calvaris, cdumez, cmarcelo, commit-queue, dbates, ews-watchlist, ysuzuki, zan | ||||||||
| Priority: | P2 | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 343904 [details]
Patch
Comment on attachment 343904 [details]
Patch
Informal r+ from me, good catch! :-)
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 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 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. Created attachment 344089 [details]
Patch
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 Created attachment 344092 [details]
Patch for landing
Comment on attachment 344092 [details] Patch for landing Clearing flags on attachment: 344092 Committed r233420: <https://trac.webkit.org/changeset/233420> All reviewed patches have been landed. Closing bug. |
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); }