Cleanup basic int type usage and type naming

[axxel]

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:

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

2. 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'.

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

[LebedevRI]

I'm wiling to bet that they are not Rust names, but linux; Assigning an Explicit Size to Data Items, linux/types.h

[axxel]

I'm wiling to bet that they are not Rust names, but linux

Meaning what exactly in the context of my question? If you have an opinion about 1) and/or 2), please tell me about it.

[LebedevRI]

No, at this point i'm unable to state my opinion/choice. But, in either case the changes will be rather intrusive so it will be better to not rush changing it and creating a pr.

[axxel]

No, at this point i'm unable to state my opinion/choice.

Ok.

But, in either case the changes will be rather intrusive so it will be better to not rush changing it and creating a pr.

That is why I put this issue together first. The influence of the size_t choice is actually not that intrusive and also more important to me, as it is actually conveying information. The choices in 1 are merely cosmetic. You also introduced code using size_t recently. Can you express an opinion on that?

[LebedevRI]

The influence of the size_t choice is actually not that intrusive and also more important to me, as it is actually conveying information. The choices in 1 are merely cosmetic. You also introduced code using size_t recently. Can you express an opinion on that?

Sure. Using anything other than size_t for anything that means size is just wrong. I.e. buffer size, multiplication of the dimensions of image, size of some buffer, etc - should use size_t.

[axxel]

Very good. I'll do that then (the consistent size_t introduction).

[LebedevRI]

Okay, as long as it is less than 500LOC diff :)

[axxel]

This turned out to be less of a no-brainer than I thought: simply switching from a 32bit size_type to a 64bit one introduces overload resolution issues. The problem is, that Tiff is inherently 32bit based. Everything that is a size or an offset is 32bit. So it makes absolute sense to have a 32bit type for those values.

I'd still like to encourage a wider spread distinction between a 'random' unsigned int and a size/offset value. Next idea was to simply introduce a using size_t = uint32_t;. But this effectively kills client code using the std namespace and we already agreed that using std names in RawSpeed is bad.

So I am back to sticking either to the Buffer::size_type variant or introduce a 'global' non-standard name like s32, s32_t, size32, size32_t. So we are sort of back to question 1... The ClassName::size_type has the issue that its verbosity could lead people to start using other types in new code (like size_t) "as long as it compiles..." and if the compiler complains, it is easy to add a c-style cast here and there, which is the opposite of what I intend to achieve. :-/

If you have a preference or some other thoughts that are relevant to the subject, please let me know.

[LebedevRI]

So if we pick the linux/rust types, what are the PRINTF(3) specifiers for them?

[axxel]

So if we pick the linux/rust types, what are the PRINTF(3) specifiers for them?

The exact same as with the current type names? My intention with 1) was simply to 'beautify' the names.

[axxel]

Because your question makes me suspect you had a different understanding of what exactly I was talking about in section 1):

I'm suggesting to replace using ushort16 = unsigned short; with either

• using u16 = unsigned short; or
• using uint16_t = unsigned short;

Instead of the latter, we can simply #include <cstdint> . Either option has absolutely no effect on the semantic/behavior of the code. It is a simple matter of taste and I'd prefer the shorter version. But the other is fine, too. What we have right now is not.

Depending on the above decision, I'd add either a s32 type or a size32_t type to be used for the 32bit size/offset variables.