Re: [PATCH] apply: Avoid ambiguous pointer provenance for CHERI/Arm's Morello

Jessica Clarke [off-list ref] writes:

quoted

I actually wonder if it results in code that is much easier to
follow if we did:

* Introduce an "enum apply_symlink_treatment" that has
  APPLY_SYMLINK_GOES_AWAY and APPLY_SYMLINK_IN_RESULT as its
  possible values;

* Make register_symlink_changes() and check_symlink_changes()
  work with "enum apply_symlink_treatment";

* (optional) stop using string_list() to store the symlink_changes;
  use strintmap and use strintmap_set() and strintmap_get() to
  access its entries, so that the ugly implementation detail
  (i.e. "the container type we use only has a (void *) field to
  store caller-supplied data, so we cast an integer and a pointer
  back and forth") can be safely hidden.

Those would be better if you want a less-minimal change.

The first two at least would make an understandable change, as
opposed to the code as posted, which is totally opaque why we need
to take size_t and cast it to uintptr_t in the first place.

I have no confidence in myself that I would not accept a future
patch that reverts the change made by this patch happily.  I usually
try to be careful to go back to the originating commit by running
"blame" on the preimage and may find your commit, but that's only
"usually".  If I were to work only with the file contents after
applying this patch, because no clue ...

	ent->util = (void *)((uintptr_t)what | ((uintptr_t)ent->util));

... on this line of code hints why we must be called with size_t and
have to cast it here, instead of working with uintptr_t throughout,
I am reasonably sure I'd happily take such a patch and break your
"fix" here.

If we make the code pass around the enum, have a temporary variable
of the same enum type to compute the next value we stuff in
ent->util, and make an assignment of that enum value to ent->util,
it is much less likely that I'd blindly accept a patch to take us
back to deal with uintptr_t directly.