Skip to content

Commit

Permalink
Update file_descriptor.read_bytes do not alloc.
Browse files Browse the repository at this point in the history
- `read_bytes` takes in Span[Byte] buffer as opposed to allocating
  to avoid n+1 alloc. issues etc.

- Remove c_str_ptr type from ffi module since it is not a convention set
  by C lang., it will potentially get added back in at a later date
  after discussions and under its own PR

- Doc string updates to libC funcs.

Co-authored-by: Connor Gray <accounts@connorgray.com>
Signed-off-by: Hristo (Izo) G. <53634432+izo0x90@users.noreply.github.com>
  • Loading branch information
izo0x90 and ConnorGray committed Feb 26, 2025
1 parent 62ab625 commit bba987f
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 27 deletions.
38 changes: 20 additions & 18 deletions stdlib/src/builtin/file_descriptor.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ f.close()
from sys import os_is_macos, os_is_linux
from sys._amdgpu import printf_append_string_n, printf_begin
from sys.ffi import c_ssize_t, external_call
from sys.info import is_amd_gpu, is_nvidia_gpu
from sys.info import is_amd_gpu, is_gpu, is_nvidia_gpu

from builtin.io import _printf
from builtin.os import abort
Expand Down Expand Up @@ -89,33 +89,35 @@ struct FileDescriptor(Writer):
)

@always_inline
fn read_bytes(mut self, size: UInt) raises -> List[Byte]:
fn read_bytes(mut self, mut buffer: Span[Byte, _]) raises -> UInt:
"""
Read a number of bytes from the file.
Args:
size: Number of bytes to read.
buffer: Span[Byte] of length n where to store read bytes. n = number of bytes to read.
Returns:
A list of bytes.
Actual number of bytes read.
"""

@parameter
if is_nvidia_gpu():
constrained[False, "Nvidia GPU read bytes not implemented"]()
return abort[List[Byte]]()
elif is_amd_gpu():
constrained[False, "AMD GPU read bytes not implemented"]()
return abort[List[Byte]]()
elif os_is_macos() or os_is_linux():
var ptr = UnsafePointer[Byte].alloc(size)
constrained[
not is_gpu(), "`read_bytes()` is not yet implemented for GPUs."
]()

read = external_call["read", c_ssize_t](self.value, ptr, size)

return List[Byte](ptr=ptr, length=read, capacity=read)
@parameter
if os_is_macos() or os_is_linux():
read = external_call["read", c_ssize_t](
self.value, buffer.unsafe_ptr(), len(buffer)
)
if read < 0:
raise Error("Failed to read bytes.")
return read
else:
constrained[False, "Unknown platform read bytes not implemented"]()
return abort[List[Byte]]()
constrained[
False,
"`read_bytes()` is not yet implemented for unknown platform.",
]()
return abort[UInt]()

fn write[*Ts: Writable](mut self, *args: *Ts):
"""Write a sequence of Writable arguments to the provided Writer.
Expand Down
17 changes: 11 additions & 6 deletions stdlib/src/sys/_libc.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ functionality in the rest of the Mojo standard library.
"""

from sys import os_is_windows
from sys.ffi import OpaquePointer, c_char, c_int, c_size_t, c_str_ptr
from sys.ffi import OpaquePointer, c_char, c_int, c_size_t

from memory import UnsafePointer

Expand Down Expand Up @@ -107,19 +107,23 @@ fn dup(oldfd: c_int) -> c_int:


@always_inline
fn execvp(file: UnsafePointer[c_char], argv: UnsafePointer[c_str_ptr]) -> c_int:
fn execvp(
file: UnsafePointer[c_char], argv: UnsafePointer[UnsafePointer[c_char]]
) -> c_int:
"""[`execvp`](https://pubs.opengroup.org/onlinepubs/9799919799/functions/exec.html)
— execute a file.
Args:
argv: The c_str_ptr array must be terminated with a NULL pointer.
file: NULL terminated UnsafePointer[c_char] (C string), containing path to executable.
argv: The UnsafePointer[c_char] array must be terminated with a NULL pointer.
"""
return external_call["execvp", c_int](file, argv)


@always_inline
fn vfork() -> c_int:
"""[`vfork()`](https://pubs.opengroup.org/onlinepubs/009696799/functions/vfork.html)."""
"""[`vfork()`](https://pubs.opengroup.org/onlinepubs/009696799/functions/vfork.html).
"""
return external_call["vfork", c_int]()


Expand All @@ -142,7 +146,8 @@ fn kill(pid: c_int, sig: c_int) -> c_int:

@always_inline
fn pipe(fildes: UnsafePointer[c_int]) -> c_int:
"""[`pipe()`](https://pubs.opengroup.org/onlinepubs/9799919799/functions/pipe.html) — create an interprocess channel."""
"""[`pipe()`](https://pubs.opengroup.org/onlinepubs/9799919799/functions/pipe.html) — create an interprocess channel.
"""
return external_call["pipe", c_int](fildes)


Expand Down
3 changes: 0 additions & 3 deletions stdlib/src/sys/ffi.mojo
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,6 @@ alias c_float = Float32
alias c_double = Float64
"""C `double` type."""

alias c_str_ptr = UnsafePointer[c_char]
"""C `*char` type"""

alias OpaquePointer = UnsafePointer[NoneType]
"""An opaque pointer, equivalent to the C `void*` type."""

Expand Down

0 comments on commit bba987f

Please sign in to comment.