darktable-org / darktable-org/rawspeed
Cleanup basic int type usage and type naming
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 456
- Forks
- 147
- PR merge metrics
- No merged PRs in 30d
Description
I have 3 issues with the custom types specified here: https://github.com/darktable-org/rawspeed/blob/develop/src/librawspeed/common/Common.h#L35-L41
- Their naming contains the size info twice, e.g.
ushort16and the names (strings) are not equally long.
I like to suggest two alternative variants:
- Rust names (
u32, i16, etc.) <cstdint>: http://en.cppreference.com/w/cpp/types/integer
I'd prefer the short Rust names, especially because the std names are longer than what we have right now and the _t postfix looks very 'c-ish' to me. I'd also suggest to keep the word byte and even use that term as a type name for the unsigned char case. That name even almost made it into c++17. The idea is to distinguish between simple data bytes and characters (as in bits of a string).
- The usage of the
uint32for offets/sizes is 'non-standard'
I'd like to go over the interfaces that I touched and simply use size_t for that, even in the cases where I introduced the size_type name that can be found in the std:: containers. I always found that part a bit 'over engineered'.
- Their definition is potentially wrong (depending on the data model, see http://en.cppreference.com/w/cpp/language/types). E.g. the tiff code breaks if 'unsigned int' would be 16bits wide. This should be non-controversial and is trivial to fix.
@LebedevRI What is your opinion on 1) and 2)?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with the custom type definitions in src/librawspeed/common/Common.h at lines 35–41, then trace their use in interfaces and the TIFF code. Compare the proposed naming alternatives, review offset and size parameters for size_t usage, and check the definitions against C++ data-model guarantees. Done means the chosen type naming, usage, and definitions are applied consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100