Clean up of warnings (tested on clang). #9
Conversation
|
Hi Paolo. Unfortunately ssize_t does not exist in some Windows compilers. Just yesterday I just had this issue on my uthash project where we removed ssize_t. In order to merge this, we'd need to have some kind of ifdef that provided an alternative for Windows. I don't use clang myself, but I've had patches before, that were just there to quiet clang warnings. Unfortunately they tend to obfuscate the code and make it more fragile. For example the first case where an int being replaced by ssize_t is misleading, because I know that the int is in fact a small positive number (so maybe it should be unsigned int - but what benefit is there in using size_t?). But I have no way to tell clang that. In other cases, explicit types like uint32_t are used in tpl because tpl casts between fixed-length buffers from the the binary tpl image to them. So that is how we guarantee we can memcpy between the tpl image and memory. Does clang have features where you can give it hints that help it understand why certain types are used the way they are? |
| @@ -185,7 +185,7 @@ static int tpl_mmap_file(char *filename, tpl_mmap_rec *map_rec); | |||
| static int tpl_mmap_output_file(char *filename, size_t sz, void **text_out); | |||
| static int tpl_cpu_bigendian(void); | |||
| static int tpl_needs_endian_swap(void *); | |||
| static void tpl_byteswap(void *word, int len); | |||
| static void tpl_byteswap(void *word, ssize_t len); | |||
There was a problem hiding this comment.
The len here is always a small positive number like 4 or 8.
|
Hi Troy, |
Accepted Troy's suggestions. In tpl_dump(), cast to int from a write() return value is performed after checking for INT_MAX. Also, avoiding use of ssize_t (in favour to size_t or long+cast) to be windows friendly.
|
I resolved the conflict but am unable to push to your branch. Can you please enable. |
| @@ -302,7 +306,7 @@ char *calc_field_addr(tpl_node *parent, int type,char *struct_addr, int ordinal) | |||
| align_sz = tpl_types[type].sz; | |||
| break; | |||
| } | |||
| offset = ((uintptr_t)prev->addr - (uintptr_t)struct_addr) | |||
| offset = ((size_t)prev->addr - (size_t)struct_addr) | |||
There was a problem hiding this comment.
I rather see the ptrdiff_t type here. See https://www.viva64.com/en/a/0050/ for example.
| static const char tpl_S_fmt_chars[] = "iucsfIUjv#$()p"; /* valid within S(...) */ | ||
| #pragma clang diagnostic pop |
There was a problem hiding this comment.
VS reports warning C4068: Unknown Pragma, thus you probably should wrap the pragma in #ifdef.
I tried to cleanup all of the warnings for implicit conversions. I understand that it can be a non-trivial task, for the formability issues related to type conversion can be hard to spot at a first look on the code. For this reason, I am not 100% sure I've done all the necessary edits to your code.
Nonetheless, I think that checking and cleaning up type conversion warnings would be a desirable task, giving users some larger confidence in the code reliability.
Of course, thanks for sharing such a helpful piece of software.