Skip to content

Conversation

@Smjert
Copy link
Member

@Smjert Smjert commented Sep 3, 2024

  • Simplify readFile interface to only support reading full files

  • Remove the choice of reading in a blocking way, since we want to prevent hanging

  • Remove the dry run read, since it was not useful

  • Split the implementation between two overloads, where one supports returning the whole file contents in one go, while the other calls back a predicate everytime some data is read, up to a fixed block size (as with the previous implementation)

@Smjert Smjert added the core label Sep 3, 2024
@Smjert Smjert force-pushed the stefano/improvement/readfile-nonblock branch 2 times, most recently from af24756 to 00ffd41 Compare September 3, 2024 19:04
- Simplify readFile interface to only support reading full files

- Remove the choice of reading in a blocking way,
  since we want to prevent hanging

- Remove the dry run read, since it was not useful

- Split the implementation between two overloads,
  where one supports returning the whole file contents in one go,
  while the other calls back a predicate everytime some data is read,
  up to a fixed block size (as with the previous implementation)
@Smjert Smjert force-pushed the stefano/improvement/readfile-nonblock branch from 00ffd41 to 0d5203b Compare September 3, 2024 19:22
@michael-myers
Copy link
Contributor

Nice! I know this has been talked about for a long time.

@Smjert Smjert marked this pull request as ready for review September 17, 2024 18:19
@Smjert Smjert requested review from a team as code owners September 17, 2024 18:19
@directionless directionless merged commit 35973ec into osquery:master Oct 8, 2024
@Smjert Smjert deleted the stefano/improvement/readfile-nonblock branch October 8, 2024 13:20
zwass added a commit that referenced this pull request Jun 3, 2025
A refactor of the `readFile` function (in #8410) caused all parsing of shortcut files to fail. The test had a bug that caused the relevant checks to be skipped.

Fixes #8599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants