This is an archive of the discontinued LLVM Phabricator instance.

[scudo] Simplify internal names (NFC)
ClosedPublic

Authored by cryptoad on Jul 18 2018, 1:04 PM.

Details

Summary

There is currently too much redundancy in the class/variable/* names in Scudo:

  • we are in the namespace __scudo, so there is no point in having something named ScudoX to end up with a final name of __scudo::ScudoX;
  • there are a lot of types/* that have Allocator in the name, given that Scudo is an allocator I figure this doubles up as well.

So change a bunch of the Scudo names to make them shorter, less redundant, and
overall simpler. They should still be pretty self explaining (or at least it
looks so to me).

The TSD part will be done in another CL (eg __scudo::ScudoTSD).

Diff Detail

Event Timeline

cryptoad created this revision.Jul 18 2018, 1:04 PM
Herald added subscribers: Restricted Project, delcypher. · View Herald TranscriptJul 18 2018, 1:04 PM
alekseyshl added inline comments.Jul 18 2018, 1:17 PM
lib/scudo/scudo_allocator.h
112

I'd keep PrimaryAllocator and SecondaryAllocator, PrimaryT is just puzzling to see in the code.

cryptoad updated this revision to Diff 156145.Jul 18 2018, 1:23 PM
cryptoad marked an inline comment as done.

Keeping {Primary|Secondary}Allocator for clarity.

Well, now it is not consistent. Why some types end with T and some not (AllocatorCacheT vs PrimaryAllocator)? Why combined allocator is called Backend and struct Allocator is not called Frontend?

To be honest, I don't have better names in mind at the moment. I kind of get used to the current naming, so I might be biased, but it is more or less a matter of taste. If you keep it consistent, I can live with your original PrimaryT and SecondaryT. Sorry for the churn.

Thanks. No worries I'll change it back!

To be honest, I don't have better names in mind at the moment. I kind of get used to the current naming, so I might be biased, but it is more or less a matter of taste. If you keep it consistent, I can live with your original PrimaryT and SecondaryT. Sorry for the churn.

cryptoad updated this revision to Diff 156168.Jul 18 2018, 3:49 PM

Bringing back {Primary|Secondary}T.

alekseyshl accepted this revision.Jul 19 2018, 10:50 AM
This revision is now accepted and ready to land.Jul 19 2018, 10:50 AM
This revision was automatically updated to reflect the committed changes.