WIP: Patch in PR #25 and fix for use after close / double close. #37
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
…>file' in _codes_index_add_file(). Introduce 'index_next' field in grib_file so that an object of the kind can appear simualtaneously both in index->files chain and file_pool.first chain. Introduce 'dependants' field in grib_file so that a 'grib_file' object derived from another can be chained via the field. File pointer (grib_file->handle) will not be closed unless there is no dependants. Close 'grib_file's on index->files chain in grib_index_delete(). Remove 'size' member of grib_file_pool as it is not utilized. Reduced scope of some internal functions to static. Close 'grib_flie's on index->files link in 'grib_index_delete()'. Expose 'grib_file_pool_clean(int *err)' to internal tools and remove 'grib_file_close_all()'. Rename grib_get_file() to grib_get_or_create_file() to describe what the function does more appropriety.
shinji-s
changed the title
|
Ouch. Now locally running the extra tests... |
…only once for each 'file_id' value. Don't assume files[i] gets copiied and nullified while reading index file.
Manage 'id' per grib_index by introducinng 'filed_id_seq' member. Add 'int * created' arg to grip_open_file() so that we can tell if the 'grib_file' object is newly created. Have grib_index_add_new_key() use a newly created 'grib_file' object as is or create a duplicate otherwise. Rename 'next' to 'newkey' for clarity in grib_index_add_new_key(). Initialize grib_file.id with -1. Remove grib_index_key.count as it is not used. Rename grib_index_new_key() to grib_index_add_new_key() for clarity. Release previously allocated object before returning due to memory allocation error. Introduce LIST_APPEND macro. Rename 'p' and 'q' to more descriptive ones in 'grib_index_new()'. Make 'grib_read_field_tree()' static.
|
Does making the 'file_pool' a per 'grib_index' object make sense? Or should it be per 'grib_contex' object? |
|
I'm beginning to realize this fix was set in wrong direction from the very start. 'grib_file' is serving multiple purposes such as helping file_pool management and file reference tracking in grib_field. They don't have to use the same data structure and they should not for conceptual clarity although the underlining implementation may be shared. But what I have done is to couple them even tighter. I guess I've have to restart from scratch. Therefore I'll keep this PR open for a few days and then close it. |
Add assertion on grib_file.id.
|
Closing per the above coment. |
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.
Let's see if CI tests pass or fail.
@shahramn, please let me know what you think.