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

Jump to bottom

Removes implementation of my_strdup() from the multi VFD #527

Merged
merged 11 commits into from
Mar 31, 2021
Merged

Removes implementation of my_strdup() from the multi VFD #527

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
ba9deae
Committing clang-format changes
github-actions[bot] Mar 19, 2021
00f9750
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 21, 2021
e997283
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 22, 2021
3eac585
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 23, 2021
71643b7
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 24, 2021
d242911
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 24, 2021
bc70f95
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 26, 2021
3d7ad8e
Merge remote-tracking branch 'canonical/develop' into develop
derobins Mar 26, 2021
aeb16b7
Merge branch 'develop' of https://github.com/derobins/hdf5 into develop
derobins Mar 26, 2021
78ac479
Removes my_strdup() from the multi VFD
derobins Mar 27, 2021
e109ab2
Use strdup directly when memory sanity checks are off
derobins Mar 30, 2021
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
44 changes: 11 additions & 33 deletions src/H5FDmulti.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,15 @@
#define TRUE 1
#endif

/* Windows doesn't like some POSIX names and redefines them with an
* underscore
*/
#ifdef _WIN32
#define my_strdup _strdup
#else
#define my_strdup strdup
#endif

Copy link
Member Author

Choose a reason for hiding this comment

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

We can't just define strdup as _strdup as the former also exists on Windows. The problem with just using strdup directly is that it's deprecated and raises warnings.

Copy link
Contributor

Choose a reason for hiding this comment

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

strdup isn't deprecated, it wasn't part of the C Standard (until the upcoming C23: https://en.cppreference.com/w/c/experimental/dynamic/strdup). We can't rely on it being available on all platforms yet.

Copy link
Member Author

Choose a reason for hiding this comment

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

The non-underscore name is deprecated on Windows. From MSDN:

The Microsoft-implemented POSIX function names strdup and wcsdup are deprecated aliases for the _strdup and _wcsdup functions. By default, they generate Compiler warning (level 3) C4996. The names are deprecated because they don't follow the Standard C rules for implementation-specific names. However, the functions are still supported.

We recommend you use _strdup and _wcsdup instead. Or, you can continue to use these function names, and disable the warning. For more information, see Turn off the warning and POSIX function names.

Copy link
Contributor

Choose a reason for hiding this comment

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

The non-underscore name is deprecated on Windows. From MSDN:

The Microsoft-implemented POSIX function names strdup and wcsdup are deprecated aliases for the _strdup and _wcsdup functions. By default, they generate Compiler warning (level 3) C4996. The names are deprecated because they don't follow the Standard C rules for implementation-specific names. However, the functions are still supported.

We recommend you use _strdup and _wcsdup instead. Or, you can continue to use these function names, and disable the warning. For more information, see Turn off the warning and POSIX function names.

Ah, sorry! I misinterpreted your comment and thought it was about strdup itself. My comment was toward that direction: strdup isn't (yet) part of the C Standard, so this VFD may need the my_strdup() code for a while longer.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, the library doesn't really have an option if you don't have strdup. Either you have POSIX strdup or Windows _strdup or you are out of luck. In the "out of luck" case, we declare a strdup function exists, but we don't provide an implementation. The code in H5private.h says strdup is non-POSIX, but that's probably pre-2001 code.

/* Loop through all mapped files */
#define UNIQUE_MEMBERS_CORE(MAP, ITER, SEEN, LOOPVAR) \
{ \
Expand Down Expand Up @@ -103,9 +112,8 @@ typedef struct H5FD_multi_dxpl_t {
} H5FD_multi_dxpl_t;

/* Private functions */
static char *my_strdup(const char *s);
static int compute_next(H5FD_multi_t *file);
static int open_members(H5FD_multi_t *file);
static int compute_next(H5FD_multi_t *file);
static int open_members(H5FD_multi_t *file);

/* Callback prototypes */
static herr_t H5FD_multi_term(void);
Expand Down Expand Up @@ -171,36 +179,6 @@ static const H5FD_class_t H5FD_multi_g = {
H5FD_FLMAP_DEFAULT /*fl_map */
};

/*-------------------------------------------------------------------------
* Function: my_strdup
*
* Purpose: Private version of strdup()
*
* Return: Success: Ptr to new copy of string
*
* Failure: NULL
*
* Programmer: Robb Matzke
* Friday, August 13, 1999
*
*-------------------------------------------------------------------------
*/
static char *
my_strdup(const char *s)
{
char * x;
size_t str_len;

if (!s)
return NULL;
str_len = strlen(s) + 1;
if (NULL == (x = (char *)malloc(str_len)))
return NULL;
memcpy(x, s, str_len);

return x;
}

/*-------------------------------------------------------------------------
* Function: H5FD_multi_init
*
Expand Down
15 changes: 13 additions & 2 deletions src/H5MM.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,11 +449,17 @@ H5MM_xstrdup(const char *s)

FUNC_ENTER_NOAPI(NULL)

#if defined H5_MEMORY_ALLOC_SANITY_CHECK
if (s) {
if (NULL == (ret_value = (char *)H5MM_malloc(HDstrlen(s) + 1)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
HDstrcpy(ret_value, s);
} /* end if */
}
#else
if (s)
if (NULL == (ret_value = HDstrdup(s)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "string duplication failed")
#endif

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down Expand Up @@ -483,10 +489,15 @@ H5MM_strdup(const char *s)
FUNC_ENTER_NOAPI(NULL)

if (!s)
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "null string")
HGOTO_ERROR(H5E_ARGS, H5E_BADVALUE, NULL, "NULL string not allowed")
#if defined H5_MEMORY_ALLOC_SANITY_CHECK
if (NULL == (ret_value = (char *)H5MM_malloc(HDstrlen(s) + 1)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed")
HDstrcpy(ret_value, s);
#else
if (NULL == (ret_value = HDstrdup(s)))
HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "string duplication failed")
#endif

done:
FUNC_LEAVE_NOAPI(ret_value)
Expand Down
15 changes: 3 additions & 12 deletions src/H5private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,6 +1484,9 @@ H5_DLL void HDsrand(unsigned int seed);
#ifndef HDstrcspn
#define HDstrcspn(X, Y) strcspn(X, Y)
#endif /* HDstrcspn */
#ifndef HDstrdup
#define HDstrdup(S) strdup(S)
#endif /* HDstrdup */
#ifndef HDstrerror
#define HDstrerror(N) strerror(N)
#endif /* HDstrerror */
Expand Down Expand Up @@ -1672,18 +1675,6 @@ H5_DLL int HDvasprintf(char **bufp, const char *fmt, va_list _ap);
#define HDwrite(F, M, Z) write(F, M, Z)
#endif /* HDwrite */

/*
* And now for a couple non-Posix functions... Watch out for systems that
* define these in terms of macros.
*/
#if !defined strdup && !defined H5_HAVE_STRDUP
extern char * strdup(const char *s);
#endif

#ifndef HDstrdup
#define HDstrdup(S) strdup(S)
#endif /* HDstrdup */

/* Macro for "stringizing" an integer in the C preprocessor (use H5_TOSTRING) */
/* (use H5_TOSTRING, H5_STRINGIZE is just part of the implementation) */
#define H5_STRINGIZE(x) #x
Expand Down