-
Notifications
You must be signed in to change notification settings - Fork 103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
K&R utils/** and const-correctness (part 1) #352
Open
dlmiles
wants to merge
39
commits into
RTimothyEdwards:master
Choose a base branch
from
dlmiles:master-upstream-20241018-utils
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
K&R utils/** and const-correctness (part 1) #352
dlmiles
wants to merge
39
commits into
RTimothyEdwards:master
from
dlmiles:master-upstream-20241018-utils
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is mainly to modify the function signature to accept const data pointers to keys. Patch required to ease use of const by API consumers.
This then rippled through into callers
extern int Lookup(const char *str, const char * const *table); extern int LookupAny(char, const char * const *); extern int LookupFull(const char *, const char **); extern int LookupStruct(const char *str, const LookupTable *table_start, int size); extern int LookupStructFull(const char *str, const char * const *table, int size);
txInput.c: LookupStructFull() constify call-site DBlabel.c: LookupStructFull() constify call-site
GeoDisjoint() handles this as 'bool' type internally so make all consumer call-sites consistent with this type.
irouter/irTestCmd.c: LookupStruct() constify call-site irouter/irCommand.c: LookupStruct() constify call-site mzrouter/mzTestCmd.c: LookupStruct() constify call-site mzrouter/mzTech.c: LookupStruct() constify call-site router/rtrCmd.c: LookupStruct() constify call-site plow/PlowTest.c: LookupStruct() constify call-site plow/PlowTech.c: LookupStruct() constify call-site plot/plotVers.c: LookupStruct() constify call-site grouter/grouteTest.c: LookupStruct() constify call-site garouter/gaTest.c: LookupStruct() constify call-site extract/ExtTest.c: LookupStruct() constify call-site extract/ExtTech.c: LookupStruct() constify call-site extflat/EFread.c: LookupStruct() constify call-site DRCtech.c: LookupStruct() constify call-site debugFlags.c: LookupStruct() constify call-site CmdSubrs.c: LookupStruct() constify call-site geometry.c: LookupStruct() constify call-site set.c: LookupStruct() constify call-site
lef/lefRead.c LookupFull() constify call-site lef/defRead.c LookupFull() constify call-site graphics/grDStyle.c: LookupFull() constify call-site
ext2spice/ext2spice.c: Lookup() constify call-site ext2sim/ext2sim.c: Lookup() constify call-site windows/windCmdAM.c: Lookup() constify call-site Tcl_SetResult() uses cast to remove 'const' from type, the pointer is only used to take a copy of the data, the lack of 'const' is due to Tcl heritage when supporting C89 era compilers. TCL9 appears to fix this, in that the macro used ends up at Tcl_NewStringObj() which has 'const' here.
This commit related to the dynamic creation of data that is used to parse commands and options via Lookup. windows/windows.h: Lookup() constify call-site tcltk/tclmagic.c: Lookup() constify call-site graphics/W3Dmain.c: Lookup() constify call-site windows/windSend.c: Lookup() constify call-site windows/windMain.c: Lookup() constify call-site windows/windInt.h: Lookup() constify call-site textio/txMain.c: Lookup() constify call-site
plow/PlowCmd.c: Lookup() constify call-site plot/plotVers.c: Lookup() constify call-site plot/plotMain.c: Lookup() constify call-site plot/plotCmd.c: Lookup() constify call-site netmenu/NMnetlist.c: Lookup() constify call-site netmenu/NMcmdLZ.c: Lookup() constify call-site netmenu/NMcmdAK.c: Lookup() constify call-site lef/lefTech.c: Lookup() constify call-site lef/lefCmd.c: Lookup() constify call-site irouter/irRoute.c: Lookup() constify call-site irouter/irCommand.c: Lookup() constify call-site router/rtrCmd.c: Lookup() constify call-site resis/ResRex.c: Lookup() constify call-site gcr/gcrShwFlgs.c: Lookup() constify call-site windows/windCmdSZ.c: Lookup() constify call-site
extflat/EFantenna.c: Lookup() constify call-site drc/DRCtech.c: Lookup() constify call-site dbwind/DBWelement.c: Lookup() constify call-site database/DBtpaint.c: Lookup() constify call-site commands/CmdTZ.c: Lookup() constify call-site commands/CmdRS.c: Lookup() constify call-site commands/CmdLQ.c: Lookup() constify call-site commands/CmdFI.c: Lookup() constify call-site commands/CmdE.c: Lookup() constify call-site commands/CmdCD.c: Lookup() constify call-site commands/CmdAB.c: Lookup() constify call-site cmwind/CMWcmmnds.c: Lookup() constify call-site
When constifying there is this inconsistent quirk in this API returning 'filename' or a malloced storage. When special handling needs to be made by the caller to detect this to decide if it needs a free. This appears to done to save a strdup(). Make it always return a malloc block so the API contract is strightforward to the caller. A non-NULL return requires free() by the caller.
This function does not appear used across the codebase. No prototype exists in utils.h
This would be a bug on WIN64 and other LLP64 models.
WARNING 64bit to 32bit truncation Was directly overwriting a 32bit storage location with a 64bit value.
Dereference of 'h' after calling freeMagic(h) Found while putting in cast. Initially this is what was thought however.... freeMagic() has this one allocation to free latency, which is a matter to resolve another day.
non-static file local functions
This is a result of hash.[ch] constification
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The K&R conversion of utils/** warranted a little more attention to put in place const-correctness as the same time, due to the ripple improvement effect across all the other modules consuming them.
Attention on the HEAD~5 fix commits and the 2 commits relating to PaCheckCompressed()
The commit relating to WindGetCommandTable maybe a little more tricky as it manages to non-const creation of dynamic data to then handoff for use with Lookup family functions.
The rest should be pretty much additional of 'const' keyword.
Showing as -Wall clean on older GCC8.