Jump to content
Haru

[2016-01-06] TBL_* typedefs and BL_CAST() variants

Recommended Posts

Rationale:

This is part of a larger source cleanup project. Following the code style guidelines we decided to adopt (linux kernel guidelines), typedefs should be avoided except where necessary.

At the same time, we noticed that we have far too many explicit casts through the code, that might easily hide a coding error (for example a variable that changed type), so we're going to try and remove as many of those as possible as well.

 

Contents:

The various TBL_* typedefs are completely unnecessary (they're only needed for internal use by the BL_CAST() macro). As such, they shouldn't be used through the code, so they have been deprecated (they're not marked as deprecated yet, due to technical reasons, but you can assume they are).

Taking the chance, the BL_CAST() macro was changed to a family of macros (BL_CAST, BL_CCAST, BL_UCAST, BL_UCCAST), each with different behavior and purpose:

 

- BL_CAST(): Casts its argument to the specified type, after ensuring the block list was of the correct type. The argument must be non-constant, and it may be evaluated more than once (i.e. unsafe to use with mapit->next(x))

- BL_CCAST(): Same as BL_CAST, but it takes a const block list pointer, and returns a const object.

- BL_UCAST(): Casts its argument to the specified type, without verifying the source block list (it is the caller's care to do that). The argument is guaranteed to be evaluated only once, making it suitable to be used with mapit->next(x) or similar functions.

- BL_UCCAST(): Same as BL_UCAST(), but takes a const block list pointer, and returns a const object.

 

Impact:

Custom code may stop compiling until the appropriate changes are made. Changes are easy/trivial (mostly doable as a find and replace)

 

Details:

Any use of the TBL_* typedefs must be replaced with the corresponding struct:

 

- TBL_PC -> struct map_session_data

- TBL_NPC -> struct npc_data

- TBL_MOB -> struct mob_data

- TBL_ITEM -> struct flooritem_data

- TBL_CHAT -> struct chat_data

- TBL_SKILL -> struct skill_unit

- TBL_PET -> struct pet_data

- TBL_HOM -> struct homun_data

- TBL_MER -> struct mercenary_data

- TBL_ELEM -> struct elemental_data

 

Any explicit casts of a struct block_list to any of the above types, should be replaced with the most appropriate BL_CAST() variant (as described above).

 

Merge Date:

Wed, 6 Jan 2016 17:36:33 +0300

 

Related Pull Requests:

- #1034 - https://github.com/HerculesWS/Hercules/pull/1034 - Changed all TBL_* to the appropriate structs [Haru]

- #1024 - https://github.com/HerculesWS/Hercules/pull/1024 - Change all TBL_* to actual struct to follow unix kernel syntax [hemagx]

 

Related Commits:

- b69f7b8 - https://github.com/HerculesWS/Hercules/commit/b69f7b8 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_PC to struct map_session_data as per style guidelines [hemagx]

- 1249dc2 - https://github.com/HerculesWS/Hercules/commit/1249dc2 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_MOB to struct mob_data as per strly guidelines [hemagx]

- 3364658 - https://github.com/HerculesWS/Hercules/commit/3364658 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_HOM to struct homun_data as per style guidelines [hemagx]

- 00c95f6 - https://github.com/HerculesWS/Hercules/commit/00c95f6 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_MER to struct mercenary_data as per style guidelines [hemagx]

- b040c61 - https://github.com/HerculesWS/Hercules/commit/b040c61 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_SKILL to struct skill_data as per style guidelines [hemagx]

- 829ebdd - https://github.com/HerculesWS/Hercules/commit/829ebdd - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_PET to struct pet_data as per style guidelines [hemagx]

- 1f71f41 - https://github.com/HerculesWS/Hercules/commit/1f71f41 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_ELEM to struct elemental_data as per style guidelines [hemagx]

- 3007a37 - https://github.com/HerculesWS/Hercules/commit/3007a37 - Sat, 26 Dec 2015 11:17:14 +0200 Change all TBL_NPC to struct npc_data as per style guidelines [hemagx]

- a1b0dae - https://github.com/HerculesWS/Hercules/commit/a1b0dae - Sun, 27 Dec 2015 17:35:25 +0100 Introduced the BL_UCAST() macro as an alternative to explicit casts [Haru]

- aa574e3 - https://github.com/HerculesWS/Hercules/commit/aa574e3 - Mon, 28 Dec 2015 15:14:22 +0100 Added const variants of BL_CAST/BL_UCAST: BL_CCAST/BL_UCCAST [Haru]

- b3c722e - https://github.com/HerculesWS/Hercules/commit/b3c722e - Sun, 27 Dec 2015 18:17:24 +0100 Replaced some explicit casts with BL_UCAST/BL_UCCAST [Haru]

- f878d5e - https://github.com/HerculesWS/Hercules/commit/f878d5e - Mon, 28 Dec 2015 00:16:39 +0100 Replaced some explicit casts with BL_UCAST/BL_UCCAST [Haru]

- 2055585 - https://github.com/HerculesWS/Hercules/commit/2055585 - Mon, 28 Dec 2015 00:24:24 +0100 Replaced some map->id2sd calls with the proper map->id2XX function [Haru]

- 5db8be9 - https://github.com/HerculesWS/Hercules/commit/5db8be9 - Mon, 28 Dec 2015 02:05:09 +0100 Moved status_get_homXXX macros to status.c [Haru]

- 0e05c1e - https://github.com/HerculesWS/Hercules/commit/0e05c1e - Mon, 28 Dec 2015 15:13:02 +0100 Replaced some explicit casts with BL_UCAST [Haru]

- e3eac13 - https://github.com/HerculesWS/Hercules/commit/e3eac13 - Mon, 28 Dec 2015 15:41:36 +0100 Replaced the remaining explicit casts with BL_CAST/BL_UCAST [Haru]

- d5199ce - https://github.com/HerculesWS/Hercules/commit/d5199ce - Wed, 6 Jan 2016 17:36:33 +0300 Merge pull request #1034 from HerculesWS/bl_cast [Andrei Karas]

- f0fb759 - https://github.com/HerculesWS/Hercules/commit/f0fb759 - Wed, 6 Jan 2016 15:37:16 +0100 HPM Hooks Update [Hercules.ws]

- 0772dd0 - https://github.com/HerculesWS/Hercules/commit/0772dd0 - Wed, 6 Jan 2016 23:56:48 +0300 Fix null pointer access after previous commits. [Andrei Karas]

- 2af2132 - https://github.com/HerculesWS/Hercules/commit/2af2132 - Fri, 8 Jan 2016 16:51:59 +0100 Fixed a mapserver crash (too small allocation) [Haru]

 

Trivia:

While many of the changes here would have been possible with find and replace, I decided to review each and every of them (the actual replacement was mostly done with a vim macro, but not fully automated so I could see what the code was, and possibly do additional cleanup by hand), in order to review up some old, wrong code. It turned out that we were casting things that we didn't need to cast, or using wrong map->bl2XX() functions in several places. It costed some hours of work, but it was definitely worth it.

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...

×
×
  • Create New...

Important Information

By using this site, you agree to our Terms of Use.