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

Make server connection info file-local #86

Merged
merged 5 commits into from
Jan 16, 2024

Conversation

mattjala
Copy link
Collaborator

H5Fdelete doesn't receive any information about the file to delete besides its filename, so this may be impossible to completely refactor.

The global info (username/password in the global curl handle, global base_URL variable) haven't been removed yet, but each request to server now uses a file-local username and password.

@mattjala mattjala added the enhancement New feature or request label Oct 12, 2023
@mattjala mattjala force-pushed the conn_info_local branch 2 times, most recently from 1409474 to c132dc0 Compare October 18, 2023 21:33
@mattjala mattjala force-pushed the conn_info_local branch 2 times, most recently from ac098d2 to 9872181 Compare October 24, 2023 16:53
@mattjala mattjala marked this pull request as ready for review October 24, 2023 16:54
@mattjala
Copy link
Collaborator Author

base_URL information moved into the local files handles, and the corresponding global variable removed.

@mattjala mattjala marked this pull request as draft November 13, 2023 19:14
@mattjala mattjala marked this pull request as ready for review November 13, 2023 19:40
@@ -120,6 +121,12 @@ RV_link_create(H5VL_link_create_args_t *args, void *obj, const H5VL_loc_params_t
new_link_loc_obj = (RV_object_t *)hard_link_target_obj;
} /* end if */

if (new_link_loc_obj && ((base_URL = new_link_loc_obj->domain->u.file.server_info.base_URL) == NULL))
Copy link
Collaborator

Choose a reason for hiding this comment

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

These conditions need to be split so that assigning of base_URL doesn't depend on new_link_loc_obj being non-NULL

if (new_link_loc_obj && ((base_URL = new_link_loc_obj->domain->u.file.server_info.base_URL) == NULL))
FUNC_GOTO_ERROR(H5E_ARGS, H5E_BADVALUE, FAIL, "location object does not have valid server URL");

if (!base_URL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Duplicate check here since NULL is already checked above when base_URL is assigned

@@ -843,6 +861,33 @@ RV_object_specific(void *obj, const H5VL_loc_params_t *loc_params, H5VL_object_s
strncpy(iter_object->u.file.filepath_name, loc_obj->u.file.filepath_name,
strlen(loc_obj->u.file.filepath_name) + 1);

if ((iter_object->u.file.server_info.username =
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't iter_object here already be inheriting the file/domain from loc_obj? If so, it seems like we'd be overwriting and leaking memory by allocating the file's server_info fields here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

iter_object has the server_info struct's pointers copied into it, pointing at the allocated memory of the original file object. If we didn't allocate more memory here, the server information of the original would be freed when this copy is closed.

mattjala and others added 4 commits November 15, 2023 09:54
@jhendersonHDF jhendersonHDF merged commit efaacda into HDFGroup:master Jan 16, 2024
10 checks passed
@mattjala mattjala deleted the conn_info_local branch March 21, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants