HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Writing a chunk of a file to a socket in Swift

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swiftchunkfilewritingsocket

Problem

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  0 else {return Int64(read)}
        var writeCounter = 0
        while writeCounter  0 else {return Int64(writeResult)}
            writeCounter = writeCounter + writeResult
            writed += UInt64(writeCounter)
        }
    }
}

Solution

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  0 else {
            return bytesRead == 0 ? totalBytesWritten : nil
        }

        var bytesWritten = 0
        while bytesWritten  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.

Code Snippets

let bufferSize:Int = 1024
var read:Int = 0
let bufferSize = 1024
var read = 0
let remaining = Int(count - writed)
let readAmount = Int(min(count - totalBytesWritten, Int64(bufferSize)))
writed += UInt64(writeCounter)

Context

StackExchange Code Review Q#160992, answer score: 4

Revisions (0)

No revisions yet.