darktable-org / darktable-org/rawspeed

Cleanup basic int type usage and type naming

Open
#76 11 comments 0 reactions 0 assignees View on GitHub

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

  1. Their naming contains the size info twice, e.g. ushort16 and the names (strings) are not equally long.

I like to suggest two alternative variants:

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).

  1. The usage of the uint32 for 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'.

  1. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.