Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Remove "placeholder constants" #3131

Open
Tracked by #3248
workingjubilee opened this issue Mar 2, 2023 · 3 comments
Open
Tracked by #3248

Remove "placeholder constants" #3131

workingjubilee opened this issue Mar 2, 2023 · 3 comments
Labels
breakage-candidate C-bug Category: bug E-medium E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Milestone
1.0

Comments

@workingjubilee
Copy link
Member

It's common for C APIs to define constant values to denote the "last" value in a set of constants, like ELAST or IPPROTO_MAX. This seems prone to weird bugs, and these constants provide very marginal benefit to Rust programmers. Since we're considering some breakage anyways, we should just remove these values where we are made aware of them.

See:
@JohnTitor JohnTitor mentioned this issue Mar 3, 2023
Merged
@JohnTitor
Copy link
Member

👍, before 0.3, I'd like to deprecate these values.

@JohnTitor JohnTitor mentioned this issue Apr 19, 2023
Merged
@JohnTitor
Copy link
Member

Noticed that we have a lot of *MAX consts and deprecating manually is quite hard. We could write a script but it can cause a false positive, hmm. For now, I've edited the PR template to mention this concern.

bors added a commit that referenced this issue Apr 19, 2023
@bors
Auto merge of #3207 - JohnTitor:placeholder-pr-template, r=JohnTitor
a964608 
Mention unstable values on PR template

cc #3131
@joshtriplett joshtriplett mentioned this issue May 15, 2023
Open
19 tasks
@alexcrichton alexcrichton mentioned this issue Aug 2, 2023
Merged
@sunfishcode sunfishcode mentioned this issue Aug 9, 2023
Merged
@BrandonMFong BrandonMFong mentioned this issue Aug 18, 2023
Merged
@androm3da androm3da mentioned this issue Sep 2, 2023
Merged
@zetanumbers zetanumbers mentioned this issue Sep 8, 2023
Draft
@lcruz99 lcruz99 mentioned this issue Oct 3, 2023
Merged
@joshlf joshlf mentioned this issue Oct 25, 2023
Closed
@Ncerzzk Ncerzzk mentioned this issue Nov 17, 2023
Merged
@Kiri2002 Kiri2002 mentioned this issue Jan 14, 2024
Merged
@devnexen devnexen mentioned this issue Feb 10, 2024
Closed
@Lilit0x Lilit0x mentioned this issue Mar 14, 2024
Closed
@Amanieu
Copy link
Member

Amanieu commented Mar 15, 2024

An alternative to deleting these placeholder constants would be to still provide them but not make any guarantees about their value moving forward. This means that we would be free to change the value of these constants without it being considered a breaking change.

CI tests should specifically exclude these, although it would be nice to make CI fail only if our value for such constants is lower than the one on the system.

@asomers asomers mentioned this issue Mar 22, 2024
Merged
@snogge snogge mentioned this issue Apr 23, 2024
Merged
@mkroening mkroening mentioned this issue Apr 29, 2024
Merged
@stlankes stlankes mentioned this issue Apr 29, 2024
Closed
@youknowone youknowone mentioned this issue Apr 30, 2024
Merged
@tesuji tesuji mentioned this issue May 19, 2024
Merged
@arjunr2 arjunr2 mentioned this issue Jul 19, 2024
Closed
@ewilden ewilden mentioned this issue Jul 22, 2024
Merged
@myl7 myl7 mentioned this issue Aug 5, 2024
Open
@tgross35 tgross35 added this to the 1.0 milestone Aug 12, 2024
This was referenced Jan 6, 2025
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
@Lubov66 Lubov66 mentioned this issue Jan 10, 2025
Open
@SteveLauC SteveLauC mentioned this issue Jan 12, 2025
Open
3 tasks
@snogge snogge mentioned this issue Jan 14, 2025
Merged
3 tasks
@arjunr2 arjunr2 mentioned this issue Jan 16, 2025
Open
3 tasks
@xingxue-ibm xingxue-ibm mentioned this issue Jan 23, 2025
Merged
3 tasks
@no1wudi no1wudi mentioned this issue Jan 24, 2025
Merged
3 tasks
This was referenced Feb 3, 2025
Merged
Merged
@sunshowers sunshowers mentioned this issue Feb 12, 2025
Merged
3 tasks
@daxtens daxtens mentioned this issue Feb 13, 2025
Draft
3 tasks
@cipherself cipherself mentioned this issue Feb 13, 2025
Merged
3 tasks
@devnexen devnexen mentioned this issue Feb 14, 2025
Merged
3 tasks
@hax0kartik hax0kartik mentioned this issue Feb 15, 2025
Merged
3 tasks
@fishilico fishilico mentioned this issue Feb 18, 2025
Merged
3 tasks
@snogge snogge mentioned this issue Feb 19, 2025
Merged
3 tasks
@Berrysoft Berrysoft mentioned this issue Feb 22, 2025
Open
3 tasks
@tobhe tobhe mentioned this issue Feb 25, 2025
Merged
3 tasks
@bertptrs bertptrs mentioned this issue Feb 25, 2025
Open
3 tasks
hawkw added a commit to hawkw/libc that referenced this issue Feb 26, 2025
@hawkw
solarish: define IP_BOUND_IF and IPV6_BOUND_IF
b910481 
# Description

These socket options are currently defined only on macOS and friends,
but they exist on illumos and Solaris as well.

This commit defines these socket options on solarish operating systems.

# Sources

On Solaris, see the man page [`ip(7P)`].

I'd like to provide a link to the illumos manual pages, but apparently
https://illumos.org/man seems to be impacted by today's AWS outage.

The constants are defined in `/usr/include/netinet/in.h`:

```console
eliza@atrium ~ $ uname -a
SunOS atrium 5.11 helios-2.0.22827 i86pc i386 i86pc
eliza@atrium ~ $ rg 'IP(V6)?_BOUND_IF' /usr/include
/usr/include/netinet/in.h
978:#define   IP_BOUND_IF                0x41    /* bind socket to an ifindex       */
1311:#define  IPV6_BOUND_IF              0x41    /* bind to an ifindex */
```

[`ip(7P)`]: https://docs.oracle.com/cd/E86824_01/html/E54777/ip-7p.html

# Checklist

- [x] Relevant tests in `libc-test/semver` have been updated
- [x] No placeholder or unstable values like `*LAST` or `*MAX` are
  included (see [rust-lang#3131](rust-lang#3131))
- [x] Tested locally (`cd libc-test && cargo test --target mytarget`);
  especially relevant for platforms that may not be checked in CI
hawkw added a commit to hawkw/libc that referenced this issue Feb 26, 2025
@hawkw
solarish: define IP_BOUND_IF and IPV6_BOUND_IF
885148d 
# Description

These socket options are currently defined only on macOS and friends,
but they exist on illumos and Solaris as well.

This commit defines these socket options on solarish operating systems.

# Sources

On Solaris, see the man page [`ip(7P)`].

I'd like to provide a link to the illumos manual pages, but apparently
https://illumos.org/man seems to be impacted by today's AWS outage.

The constants are defined in `/usr/include/netinet/in.h`:

```console
eliza@atrium ~ $ uname -a
SunOS atrium 5.11 helios-2.0.22827 i86pc i386 i86pc
eliza@atrium ~ $ rg 'IP(V6)?_BOUND_IF' /usr/include
/usr/include/netinet/in.h
978:#define   IP_BOUND_IF                0x41    /* bind socket to an ifindex       */
1311:#define  IPV6_BOUND_IF              0x41    /* bind to an ifindex */
```

[`ip(7P)`]: https://docs.oracle.com/cd/E86824_01/html/E54777/ip-7p.html

# Checklist

- [x] Relevant tests in `libc-test/semver` have been updated
- [x] No placeholder or unstable values like `*LAST` or `*MAX` are
  included (see [rust-lang#3131](rust-lang#3131))
- [x] Tested locally (`cd libc-test && cargo test --target mytarget`);
  especially relevant for platforms that may not be checked in CI
@hawkw hawkw mentioned this issue Feb 26, 2025
Merged
3 tasks
@jbaublitz jbaublitz mentioned this issue Feb 27, 2025
Draft
3 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Assignees
No one assigned
Labels
breakage-candidate C-bug Category: bug E-medium E-medium Call for participation: Medium difficulty. Experience needed to fix: Intermediate.
Projects
None yet
Milestone
1.0
Development

No branches or pull requests
4 participants
@Amanieu @tgross35 @JohnTitor @workingjubilee