Skip to content

Conversation

usebeforefree
Copy link
Contributor

I have resolved an old TODO, replacing some ArrayLists in std.net with fixed size arrays. The changes have been tested with std/net/test.zig. No changes made to the test itself, since the behaviour stays the same.

usebeforefree and others added 3 commits September 16, 2025 16:38
Co-authored-by: Андрей Краевский <[email protected]>
Co-authored-by: Андрей Краевский <[email protected]>
Copy link
Collaborator

@linusg linusg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having to deal with a null state for each ns item seems a bit awkward. The BoundedArray replacement of maintaining a separate length field from https://github.com/ziglang/zig/pull/24699/files#diff-e6dad51212e62a28f43ec6ec4c94e539c08bc1e481db4de2bd49b03d85ea53e2 might be a bit nicer:

- analysis_roots: std.BoundedArray(*Package.Module, 4) = .{},
+ analysis_roots_buffer: [4]*Package.Module,
+ analysis_roots_len: usize = 0,

Copy link
Contributor

@AndrewKraevskii AndrewKraevskii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@linusg suggestion is good with u2 instead of usize.

@usebeforefree
Copy link
Contributor Author

Not sure if I should include a ns_addr_len as well, or just keep using rc.ns_len.

usebeforefree and others added 4 commits September 17, 2025 10:30
Co-authored-by: Андрей Краевский <[email protected]>
Co-authored-by: Андрей Краевский <[email protected]>
Co-authored-by: Андрей Краевский <[email protected]>
@jeroendecuyper
Copy link

jeroendecuyper commented Sep 17, 2025

Doesn't this PR throw an error if there are more then 3 nameservers defined vs glibc where it just ignores from the fourth one on?
Due to this line 1829.

@usebeforefree
Copy link
Contributor Author

Doesn't this PR throw an error if there are more then 3 nameservers defined vs glibc where it just ignores from the fourth one on? Due to this line 1829.

You are completely right, I'm just not sure how to handle it. Returning an error makes the CI fail, but silently not returning anything is deceiving, and certainly not good.

@jeroendecuyper
Copy link

To me, this either goes in either of 2 ways =>

  • similar to glibc, ignore the fourth and so on. just from compability reasons.
  • otherwise write a modern resolver and allow all nameservers to be used that are specified.

@usebeforefree
Copy link
Contributor Author

Thank you for your help and patience, the CI should pass now. :)

@andrewrk andrewrk self-requested a review September 18, 2025 02:46
@andrewrk
Copy link
Member

I'd like to be in charge of merging this please because I have some related changes in a branch to be merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants