Writing a chunk of a file to a socket in Swift

Question

I'm writing a chunk of a file from an offset to a socket.

This is my implementation and it's working OK from my tests.

Any comments or speed improvements?

private func sendChunk(_ source: Int32, _ target: Int32, _ offset: off_t, _ count: UInt64) -> Int64 {
 let bufferSize:Int = 1024
 var buffer = [UInt8](repeating: 0, count: bufferSize)
 var writed:UInt64 = 0
 var read:Int = 0
 while true {
 let remaining = Int(count - writed)
 if remaining < bufferSize {
 read = pread(source, &buffer, remaining, offset + Int64(writed))
 } else {
 read = pread(source, &buffer, buffer.count, offset + Int64(writed))
 }
 guard read > 0 else {return Int64(read)}
 var writeCounter = 0
 while writeCounter < read, writed < count {
 let writeResult = write(target, &buffer + writeCounter, read - writeCounter)
 guard writeResult > 0 else {return Int64(writeResult)}
 writeCounter = writeCounter + writeResult
 writed += UInt64(writeCounter)
 }
 }
}

Answer 1

The type annotations in

let bufferSize:Int = 1024
var read:Int = 0

are not necessary, this can be simplified to

let bufferSize = 1024
var read = 0

The variable names can be improved: read is not ideal since there is a system call read() with the same name. The past participle of "to write" is "written", not "writed", a better variable name might be totalBytesWritten since it contains the accumulated number of bytes written to the target file.

The conversion to Int in

 let remaining = Int(count - writed)

can overflow on a 32-bit architecture. If the number of bytes to read is computed as

 let readAmount = Int(min(count - totalBytesWritten, Int64(bufferSize)))

instead then it can not overflow and only one call to pread() is needed.

There is a bug in

 writed += UInt64(writeCounter)

which should be

 writed += UInt64(writeResult)

otherwise the number of bytes written is added multiple times to the total counter if multiple write() calls were necessary (and then count - writed can crash).

With the outer loop

while true { ... }

pread() is called once more if all data has been copied. That can be avoided by changing the outer loop to

while totalBytesWritten < count { ... }

Then also the logic becomes clearer and the condition in

 while writeCounter < read, writed < count { ... }

can be simplified.

Your function can only return 0 (on success) or -1 (on error), therefore the return type Int64 makes no sense.

One possibility would be to change the return type to Bool to indicate success or failure. Another possibility is to return the total number of bytes copied in the success case (which can be less than the count parameter if end-of-file was encountered), or nil on failure. Yet another possibility is to throw an error, that allows to report details on the problem to the caller.

I would not use empty external parameter names (_), a call

sendChunk(source: sfd, target: dfd, offset: 2, count: 5)

is more descriptive than

sendChunk(sfd, dfd, 2, 5)

The offset parameter could be defined with an default value zero.

Putting it all together the function could look like this:

func sendChunk(source: Int32, target: Int32, offset: off_t = 0, count: UInt64) -> UInt64? {
 let bufferSize = 1024
 var buffer = [UInt8](repeating: 0, count: bufferSize)
 var totalBytesWritten: UInt64 = 0
 while totalBytesWritten < count {
 let readAmount = Int(min(count - totalBytesWritten, UInt64(bufferSize)))
 let bytesRead = pread(source, &buffer, readAmount, offset + off_t(totalBytesWritten))
 guard bytesRead > 0 else {
 return bytesRead == 0 ? totalBytesWritten : nil
 }
 var bytesWritten = 0
 while bytesWritten < bytesRead {
 let amount = write(target, &buffer + bytesWritten, bytesRead - bytesWritten)
 guard amount > 0 else {
 return nil
 }
 bytesWritten += amount
 }
 totalBytesWritten += UInt64(bytesWritten)
 }
 return totalBytesWritten
}

Possible speed improvements: Most time will be spent in the read/write system calls. One thing that you can do is to increase the buffer size.