I have discovered a bug in NSData and I am trying to fix it (on Raspberry Pi but the bug affects any Linux OS). I can definitely compile but for some reason when I add some specific code to the file the compilation process fails with an unclear error message.
What I don't understand is that NSData.swift does compile (there is no error about it in the compile log), but its only later that it fails:
ninja: build stopped: subcommand failed.
./swift/utils/build-script: fatal error: command terminated with a non-zero exit status 1, aborting
And yet its definitely linked with my modifications to NSData.swift because if I comment parts of the code I added it doesn't fail anymore.
I don't know where to start in order to get a more precise description of the error ? Is there specific rules about what types can be used in the code for Foundation ? I use UnsafeMutableRawPointer for memory allocation, I don't see it being used elsewhere in the Foundation code, could it be that ?
For the other people: This is looping infinitely if the file prematurely closes. We've seen a scenario recently, a virtual file is being read and stat indicates a arbitrary file size.
Hello Joannis ! Yeah exactly !
Simply changing the loop breaking condition to <= compiles and at least avoids the infinite loop and lets the program explicitly fail, but I also tried to detect when NSData is given a virtual file (especially files from sysfs) in order to read it differently.
This is just a draft and is probably really perfectible but here is what I have done so far:
// If file is part of sysfs on Linux
if major(info.st_dev) == 0 {
var memoryPower = 11
var memorySize = 0b1 << memoryPower // Allocation start at 2^11, or 2048 bytes
let alignement = 1
var charPointer = UnsafeMutableRawPointer.allocate(bytes: memorySize, alignedTo: alignement)
var size = 0
var readCode = read(fd, charPointer.advanced(by: size), 1)
while readCode > 0 {
size += 1
// If the current amount of allocated memory isn't sufficient
if size > memorySize {
memoryPower += 1
memorySize = 0b1 << memoryPower
let newPointer = UnsafeMutableRawPointer.allocate(bytes: memorySize, alignedTo: alignement)
newPointer.copyBytes(from: charPointer, count: 0b1 << (memoryPower-1))
charPointer.deallocate(bytes: 0b1 << (memoryPower-1), alignedTo: alignement)
charPointer = newPointer
}
readCode = read(fd, charPointer.advanced(by: size), 1)
}
guard readCode != -1 else {
fatalError("Error reading the file")
}
let finalMemory = UnsafeMutableRawPointer.allocate(bytes: size, alignedTo: alignement)
finalMemory.copyBytes(from: charPointer, count: size)
charPointer.deallocate(bytes: 0b1 << (memoryPower), alignedTo: alignement)
if option.contains(.alwaysMapped) {
fatalError("Unimplemented")
}
return NSDataReadResult(bytes: finalMemory, length: size) { buffer, length in
buffer.deallocate(bytes: length, alignedTo: alignement)
}
} else { ... // Current foundation implementation
Reallocating memory doesn't seem really clean but if I'm right there is no other way than to read those files in order to get their actual content size.
// If file is part of sysfs on Linux
if major(info.st_dev) == 0 {
guard !options.contains(.alwaysMapped) else {
fatalError("Unimplemented")
}
var memorySize = 2048
var backingBuffer = malloc(memorySize)!
var contentSize = 0
var remaining = memorySize
var readBytesCount = read(fd, backingBuffer.advanced(by: contentSize), memorySize)
while readBytesCount > 0 {
contentSize += readBytesCount
// If current amount of allocated memory is insufficient
if contentSize > memorySize {
memorySize *= 2
if let nb = realloc(backingBuffer, memorySize) {
backingBuffer = nb
} else {
fatalError("unable to allocate backing buffer")
}
}
readBytesCount = read(fd, backingBuffer.advanced(by: contentSize), memorySize - contentSize)
}
guard readBytesCount != -1 else {
fatalError("Error reading the file")
}
if memorySize > contentSize {
backingBuffer = realloc(backingBuffer, contentSize)! // Shrink backing buffer to the real content size
}
return NSDataReadResult(bytes: backingBuffer, length: contentSize) { buffer, _ in
free(buffer)
}
} else { // For files whose size is known upfront
It's the first time ever that I try to fix a bug in Foundation I'd be interested if anyone has feedback about this code !
Thank you for this! The forums are nice but the way you can get this code reviewed and into the codebase is by making a pull request on GitHub against the Foundation repository.