Skip to content

Commit dcba769

Browse files
committed
Fix hanging and leaks in readLine
After the last line is read from the fileDescriptor the callee will call readLine once last time. If the (uninitialized) buffer just happened to contain '\r' then this method would increment bytesReceived, then decrement it (because it has '\r' in it), then decrement it agin and assign a null byte to the byte before the buffer (stepping on who knows what). Then it would return the '\r'. Then, since it received something, the callee would call readLine again, malloc would give the same buffer it did before (with the '\r') and everything would repeat. - initialize the buffer - increment bytesReceived only if a byte is actually received - don't do any work in the loop if there were no bytes received - EINTR is a recoverable error, just reread - give the actual reason for an error rather than some random string - free the buffer when a newline is found or when there is an error
1 parent 98920bf commit dcba769

File tree

1 file changed

+25
-21
lines changed

1 file changed

+25
-21
lines changed

NSFileHandleExt.m

+25-21
Original file line numberDiff line numberDiff line change
@@ -27,42 +27,46 @@ -(NSString*)readLine {
2727
char *buffer = (char*)malloc(bufferSize + 1);
2828
if (buffer == NULL)
2929
[[NSException exceptionWithName:@"No memory left" reason:@"No more memory for allocating buffer" userInfo:nil] raise];
30+
buffer[0] = '\0';
3031

31-
int bytesReceived = 0, n = 1;
32+
int bytesReceived = 0, n = 0;
3233

33-
while (n > 0) {
34-
n = read(fd, buffer + bytesReceived++, 1);
34+
while (1) {
35+
n = read(fd, buffer + bytesReceived, 1);
36+
37+
if (n == 0)
38+
break;
3539

3640
if (n < 0) {
37-
if (errno == EINTR) {
38-
n = 1;
39-
bytesReceived--;
40-
} else {
41-
[[NSException exceptionWithName:@"Socket error" reason:@"Remote host closed connection" userInfo:nil] raise];
42-
}
43-
}
41+
if (errno == EINTR)
42+
continue;
43+
44+
free(buffer);
45+
NSString *reason = [NSString stringWithFormat:@"%s:%d: read() error: %s", __PRETTY_FUNCTION__, __LINE__, strerror(errno)];
46+
[[NSException exceptionWithName:@"Socket error" reason:reason userInfo:nil] raise];
47+
}
48+
49+
bytesReceived++;
4450

4551
if (bytesReceived >= bufferSize) {
4652
// Make buffer bigger
4753
bufferSize += BUFFER_SIZE;
48-
buffer = (char*)realloc(buffer, bufferSize + 1);
54+
buffer = (char *)reallocf(buffer, bufferSize + 1);
4955
if (buffer == NULL)
5056
[[NSException exceptionWithName:@"No memory left" reason:@"No more memory for allocating buffer" userInfo:nil] raise];
5157
}
5258

53-
switch (*(buffer + bytesReceived - 1)) {
54-
case '\n':
55-
buffer[bytesReceived-1] = '\0';
56-
NSString* s = [NSString stringWithCString: buffer encoding: NSUTF8StringEncoding];
57-
if ([s length] == 0)
58-
s = [NSString stringWithCString: buffer encoding: NSISOLatin1StringEncoding];
59-
return s;
60-
case '\r':
61-
bytesReceived--;
59+
char receivedByte = buffer[bytesReceived-1];
60+
if (receivedByte == '\n') {
61+
bytesReceived--;
62+
break;
6263
}
64+
65+
if (receivedByte == '\r')
66+
bytesReceived--;
6367
}
6468

65-
buffer[bytesReceived-1] = '\0';
69+
buffer[bytesReceived] = '\0';
6670
NSString *retVal = [NSString stringWithCString: buffer encoding: NSUTF8StringEncoding];
6771
if ([retVal length] == 0)
6872
retVal = [NSString stringWithCString: buffer encoding: NSISOLatin1StringEncoding];

0 commit comments

Comments
 (0)