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.

Sign up for GitHub

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

Fix bad URL when linking VOL multiple times #98

Merged
merged 2 commits into from
Nov 10, 2023
Merged

Fix bad URL when linking VOL multiple times #98

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94354c7
Fix bad URL when linking VOL multiple times
mattjala Nov 9, 2023
e8e0324
Disable base_URL reassignment
mattjala Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 14 additions & 46 deletions src/rest_vol.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@
*/
hid_t H5_rest_id_g = H5I_UNINIT;

static hbool_t H5_rest_initialized_g = FALSE;
static hbool_t H5_rest_connection_info_initialized_g = FALSE;
static hbool_t H5_rest_initialized_g = FALSE;

/* Identifiers for HDF5's error API */
hid_t H5_rest_err_stack_g = H5I_INVALID_HID;
Expand Down Expand Up @@ -729,7 +728,7 @@ H5_rest_set_connection_information(void)
FILE *config_file = NULL;
herr_t ret_value = SUCCEED;

if (H5_rest_connection_info_initialized_g)
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.

Since base_URL is also a global, I'm surprised it doesn't have the same issues. Are we handling it differently somewhere else?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's also logic below dealing with freeing and replacing base_URL if it was already set previously. Seems like there may be some unwanted interactions with that as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the difference is because base_URL is declared extern in rest_vol.h, while H5_rest_connection_info_initialized_g was declared statically in rest_vol.c.

The logic to free and replace base_URL doesn't make sense anymore, since setting the connection information is skipped if it's already defined. I'll remove those checks. The application won't be able to change which server it connects to during the runtime of an application, but since it can only talk to one server at a time right now anyway, I think that's probably fine. Especially since #86 will allow for different files to get different connection information in a more useful way.

FUNC_GOTO_DONE(SUCCEED);

memset(&ad_info, 0, sizeof(ad_info));
Expand All @@ -746,21 +745,11 @@ H5_rest_set_connection_information(void)
URL = "0";
URL_len = 1;

if (!base_URL || (0 != (strncmp(base_URL, URL, strlen(URL))))) {
if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

/* If previous value is incorrect, reassign */
if (base_URL) {
free(base_URL);
base_URL = NULL;
}

if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

strncpy(base_URL, URL, URL_len);
base_URL[URL_len] = '\0';
}
strcpy(base_URL, URL);
}
else {
/*
Expand All @@ -770,21 +759,11 @@ H5_rest_set_connection_information(void)
*/
URL_len = strlen(URL);

if (!base_URL || (0 != (strncmp(base_URL, URL, strlen(URL))))) {

/* If previous value is incorrect, reassign */
if (base_URL) {
free(base_URL);
base_URL = NULL;
}
if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

strncpy(base_URL, URL, URL_len);
base_URL[URL_len] = '\0';
}
strcpy(base_URL, URL);
}

const char *username = getenv("HSDS_USERNAME");
Expand Down Expand Up @@ -903,21 +882,11 @@ H5_rest_set_connection_information(void)
*/
URL_len = strlen(val);

if (!base_URL || (0 != (strncmp(base_URL, val, URL_len)))) {

/* If previous value is incorrect, reassign */
if (base_URL) {
free(base_URL);
base_URL = NULL;
}
if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

if (NULL == (base_URL = (char *)RV_malloc(URL_len + 1)))
FUNC_GOTO_ERROR(H5E_VOL, H5E_CANTALLOC, FAIL,
"can't allocate space necessary for placeholder base URL");

strncpy(base_URL, val, URL_len);
base_URL[URL_len] = '\0';
}
strcpy(base_URL, val);

} /* end if */
} /* end if */
Expand Down Expand Up @@ -964,7 +933,6 @@ H5_rest_set_connection_information(void)
"must specify a base URL - please set HSDS_ENDPOINT environment variable or create a "
"config file");

H5_rest_connection_info_initialized_g = TRUE;
done:
if (config_file)
fclose(config_file);
Expand Down