Page MenuHomeFreeBSD

mandoc: Const correctness for C23
ClosedPublic

Authored by ivy on Mon, Jul 27, 10:52 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Aug 2, 10:24 PM
Unknown Object (File)
Sun, Aug 2, 8:12 PM
Unknown Object (File)
Fri, Jul 31, 6:37 PM
Unknown Object (File)
Fri, Jul 31, 5:36 AM
Unknown Object (File)
Thu, Jul 30, 5:44 AM
Unknown Object (File)
Wed, Jul 29, 5:04 PM
Unknown Object (File)
Mon, Jul 27, 11:13 PM

Details

Summary

On some platforms, e.g. Linux Clang 22.1.8 / glibc 2.43, strchr()
now implements the C23 behaviour where passing a const pointer to
strchr() also returns a const pointer. This breaks mandoc during
the bootstrap build, since it assumes the return value is always
a mutable pointer.

In read.c, make the existing temporary pointer const, and for the
mandoc_asprintf() call, add a new mutable local.

In mdoc.c and out.c, since the data is mutable and is mutated here,
remove const from the temporary pointers.

MFC after: 1 week
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ivy requested review of this revision.Mon, Jul 27, 10:52 PM

this should be upstreamed. @ziaee how do you prefer to handle that?

Thanks for tagging me! We just mail the patch to tech@mandoc.bsd.lv. I can do it if you don't want to.

This revision is now accepted and ready to land.Tue, Jul 28, 11:05 AM

Thanks for tagging me! We just mail the patch to tech@mandoc.bsd.lv. I can do it if you don't want to.

i can do that as long as i don't need to be subscribed to the list to post to it. do they prefer git send-email format?

In D58495#1342753, @ivy wrote:

Thanks for tagging me! We just mail the patch to tech@mandoc.bsd.lv. I can do it if you don't want to.

i can do that as long as i don't need to be subscribed to the list to post to it. do they prefer git send-email format?

You do need to be subscribed to the list to post it. The format they prefer is the CVS equivalent, but I use git send-email and Ingo has never objected.

This revision was automatically updated to reflect the committed changes.

You do need to be subscribed to the list to post it. The format they prefer is the CVS equivalent, but I use git send-email and Ingo has never objected.

would you mind forwarding this to the list if it's not a hassle? it would save me having to subscribe then unsubscribe, and it's a pretty trivial patch so i doubt there will be much discussion.

markmi_dsl-only.net added inline comments.
contrib/mandoc/out.c
461

Previously the declaration of the above 2 pointers indicated explicitly that they were not being used directly to mutate the state and the compiler validated such. Also, given that, the below declaration indicated it was the actual pointer of the 3 that is directly used to mutate the state.

Now, which of the 3 pointers are used to mutate the state is not explicitly declared and the code must be manually analyzed to figure that out and the compiler cannot check on the status that it previously could.

For C and C++, a specific pointer that is declared as not being used to mutate state does not mean that the referenced state needs to be invariant relative to other means of doing mutation of the same state over any scope or time frame.

So I could imagine some feedback from sending this upstream, given how it was originally written.

contrib/mandoc/out.c
461

Previously the declaration of the above 2 pointers indicated explicitly that they were not being used directly to mutate the state and the compiler validated such.

they were being used to mutate the state and the compiler was unable to validate this because the pre-C23 strchr() silently deconstifies its input. by removing const, it is now clear that they are being used to mutate state.

if you think this change is wrong, please explain why.

A note on some terminology in the summary (and for the related commits):

"const char *NAME;" is a (non-const) pointer to a const char, not a const pointer to a (non-const) char. In short: pointer to const is now being put to use, not const pointer.

[As const pointer and changes to such status are unusual, I looked into the detail --but discovered it was actually pointer to const (declared as not used to mutate) vs. pointer to non-const (declared as possibly used to mutate).]

A note on some terminology in the summary (and for the related commits):

are you going to follow me around everywhere on the Internet to post this comment?

i'm well aware of the technical difference between "const pointer to mutable T" and "mutable pointer to const T", but in colloquial language, everyone understands what "const pointer" means in this context and there is no need to "clarify" it with pointless nitpicking.

contrib/mandoc/out.c
461

If they were being used to mutate the state, does the compiler reject the source code in the C23 context when the 2 const are present? If not, the compiler disagrees with the classification of the pointers being dereferenced to do mutation (no *str= EXPRESSION; or *beg= EXPRESSION; or end=beg; [dropping const] the like for it to reject).

As error messages were not presented, I had to infer the details. If the C23 compiler context rejects declaring the 2 with const put back in, then my apologies for the noise.

In D58495#1345218, @ivy wrote:

A note on some terminology in the summary (and for the related commits):

are you going to follow me around everywhere on the Internet to post this comment?

i'm well aware of the technical difference between "const pointer to mutable T" and "mutable pointer to const T", but in colloquial language, everyone understands what "const pointer" means in this context and there is no need to "clarify" it with pointless nitpicking.

Sorry. It just happened to be that wording that caused me to look in more detail. (I have run into examples elsewhere of such wording actually being about const pointers instead.)

contrib/mandoc/out.c
461

I just got this wrong, as there is dropping const activity otherwise. Sorry for the noise. (I do now have an environment for doing compiler cross checks of such modern things before I consider commenting.)