Skip to content
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

lib/vector: reading files with 'struct Map_info*' objects is technically not const #2894

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Mar 17, 2023

Addressing -Wdeprecated-non-prototype compiler warnings and adding missing prototypes in lib/vector/Vlib/read.c, reveals API errors in lib functions passing struct Map_info * as const. Reading vector files does in fact modify the Map_info struct, therefore the const qualifier is not technically correct. Correcting this API in lib cascades through quite a few files, but functionally this doesn't change anything.

Alternative to this may be to separate the file reading variables to a separate "read object", but that will modify the API in a very drastic way and would be anything but trivial.

@nilason nilason added enhancement New feature or request C Related code is in C labels Mar 17, 2023
@nilason nilason added this to the 8.4.0 milestone Mar 17, 2023
@nilason nilason marked this pull request as draft March 17, 2023 18:58
Copy link
Contributor

@marisn marisn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine. Fixing of those two functions in read.c should fix failing tests.

lib/vector/Vlib/read.c Outdated Show resolved Hide resolved
lib/vector/Vlib/read.c Outdated Show resolved Hide resolved
Co-authored-by: Māris Nartišs <maris.gis@gmail.com>
@nilason
Copy link
Contributor Author

nilason commented Mar 20, 2023

Re: even only reading file modifies the file object. I mentioned this issue in #2809 (comment) relating a similar problem reading raster files.

@nilason nilason marked this pull request as ready for review March 21, 2023 20:12
@landam landam requested a review from marisn November 20, 2023 07:54
Copy link
Contributor

@marisn marisn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good for me. Lets merge and move on.

@nilason nilason merged commit 765b2e9 into OSGeo:main Nov 21, 2023
18 checks passed
@nilason nilason deleted the fix_more_Wdeprecated-non-prototype2 branch November 21, 2023 10:06
HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024
…lly not const (OSGeo#2894)

The qualifier is removed from the API and code using it.

Co-authored-by: Māris Nartišs <maris.gis@gmail.com>
@neteler neteler changed the title libvector : reading files with 'struct Map_info*' objects is technically not const libvector: reading files with 'struct Map_info*' objects is technically not const Jun 15, 2024
@neteler neteler changed the title libvector: reading files with 'struct Map_info*' objects is technically not const lib/vector: reading files with 'struct Map_info*' objects is technically not const Jun 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C Related code is in C enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants