Skip to content

source cleanup: remove all custom typedef structs - #5338

Closed
bagder wants to merge 2 commits into
masterfrom
bagder/remove-typedef-structs
Closed

source cleanup: remove all custom typedef structs#5338
bagder wants to merge 2 commits into
masterfrom
bagder/remove-typedef-structs

Conversation

@bagder

@bagder bagder commented May 4, 2020

Copy link
Copy Markdown
Member
  • Stick to a single unified way to use structs
  • Make checksrc complain on 'typedef struct {'
  • Allow them in tests, public headers and examples

@bagder bagder added the tidy-up label May 4, 2020
Comment thread lib/curl_md5.h Outdated
@danielgustafsson

danielgustafsson commented May 5, 2020 via email

Copy link
Copy Markdown
Member

@bagder
bagder force-pushed the bagder/remove-typedef-structs branch 8 times, most recently from 209482f to 436130e Compare May 8, 2020 08:28
@bagder
bagder marked this pull request as ready for review May 8, 2020 09:49
@bagder bagder changed the title [WIP] source cleanup: remove all custom typedef structs source cleanup: remove all custom typedef structs May 8, 2020
@bagder
bagder force-pushed the bagder/remove-typedef-structs branch from 436130e to 10bd507 Compare May 13, 2020 11:26
@bagder

bagder commented May 13, 2020

Copy link
Copy Markdown
Member Author

Buckle up. I'm going to land this asap. Just pushed a rebase and if things still are fine this will land.

@bagder
bagder force-pushed the bagder/remove-typedef-structs branch 2 times, most recently from c4f3ed4 to 6184194 Compare May 13, 2020 22:05
bagder added 2 commits May 14, 2020 08:20
 - Stick to a single unified way to use structs
 - Make checksrc complain on 'typedef struct {'
 - Allow them in tests, public headers and examples

 - Let MD4_CTX, MD5_CTX, and SHA256_CTX typedefs remain as they actually
   typedef different types/structs depending on build conditions.
@bagder
bagder force-pushed the bagder/remove-typedef-structs branch from 6184194 to 402b19d Compare May 14, 2020 06:21
@bagder

bagder commented May 14, 2020

Copy link
Copy Markdown
Member Author

Someone emailed me and asked me about the motivation for this work and here's basically my extended motivation.

Using struct spelled out in code is more clear than typedef'ed structs in the cases where "struct" is present vs not present. With struct present, we know it's a struct. Without it, we don't know what it is (by simply inspecting the name).

But I'm arguing for this change primarily for consistency. Our code has a mix of typedef'ed structs and real structs and there's nothing to hint about when we use either approach and I've come to be annoyed by not knowing. It has basically depended on the mood of the particular author which style that is used in which function.

I want the curl source code to look and feel like it was written by a single author with a single mindset on how code should look like. To make the code clean, easy to read and easy to understand - which then implies consistent. That leads to less bugs and in particular less security problems - a better product. Therefor, I want us to use structs in the code using one consistent way.

@danielgustafsson

danielgustafsson commented May 14, 2020 via email

Copy link
Copy Markdown
Member

@bagder bagder closed this in 8df4554 May 15, 2020
@bagder
bagder deleted the bagder/remove-typedef-structs branch May 15, 2020 06:55
@mkauf

mkauf commented Jun 2, 2020

Copy link
Copy Markdown
Contributor

Most autobuilds fail, because the ".checksrc" files are not found. The autobuilds perform an "out-of-tree" build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants