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

libpmi: cleanup old code and optimize client reads #5423

Merged
merged 8 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ extend-exclude = [
"t/sharness.sh",
"src/common/libutil/sha1.c",
"src/common/libutil/test/sha1.c",
"src/common/libutil/strlcpy.c",
"src/common/libutil/strlcpy.h",
"t/hwloc-data/*",
]

Expand Down
1 change: 0 additions & 1 deletion src/cmd/flux-start.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include "src/common/libutil/setenvf.h"
#include "src/common/libutil/errno_safe.h"
#include "src/common/libpmi/simple_server.h"
#include "src/common/libpmi/dgetline.h"
#include "src/common/libhostlist/hostlist.h"
#include "src/common/librouter/usock_service.h"
#include "ccan/array_size/array_size.h"
Expand Down
7 changes: 5 additions & 2 deletions src/common/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,9 @@ flux_libpmi_la_LIBADD = \
libflux-idset.la \
$(builddir)/libpmi/libpmi_client.la \
$(builddir)/libpmi/libpmi_common.la \
$(builddir)/libutil/aux.lo
$(builddir)/libutil/aux.lo \
$(builddir)/libutil/strlcpy.lo

flux_libpmi_la_LDFLAGS = \
-Wl,--version-script=$(srcdir)/libpmi.map \
-version-info 0:0:0 \
Expand All @@ -184,7 +186,8 @@ flux_libpmi2_la_LIBADD = \
libflux-idset.la \
$(builddir)/libpmi/libpmi_client.la \
$(builddir)/libpmi/libpmi_common.la \
$(builddir)/libutil/aux.lo
$(builddir)/libutil/aux.lo \
$(builddir)/libutil/strlcpy.lo
flux_libpmi2_la_LDFLAGS = \
-Wl,--version-script=$(srcdir)/libpmi2.map \
-version-info 0:0:0 \
Expand Down
4 changes: 1 addition & 3 deletions src/common/libpmi/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ libpmi_common_la_SOURCES = \
pmi_strerror.c \
pmi_strerror.h \
keyval.c \
keyval.h \
dgetline.c \
dgetline.h
keyval.h

libpmi_client_la_SOURCES = \
simple_client.c \
Expand Down
51 changes: 0 additions & 51 deletions src/common/libpmi/dgetline.c

This file was deleted.

21 changes: 0 additions & 21 deletions src/common/libpmi/dgetline.h

This file was deleted.

14 changes: 10 additions & 4 deletions src/common/libpmi/keyval.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# include "config.h"
#endif
#include <stdlib.h>
#include <limits.h>
#include <string.h>
#include <ctype.h>
#include <errno.h>
Expand All @@ -36,12 +37,14 @@ int keyval_parse_uint (const char *s, const char *key, unsigned int *val)
{
const char *cp = parse_val (s, key);
char *endptr;
unsigned int i;
unsigned long i;
if (!cp)
return EKV_NOKEY;
errno = 0;
i = strtoul (cp, &endptr, 10);
if (errno != 0 || (*endptr && !isspace (*endptr)))
if (errno != 0
|| (*endptr && !isspace (*endptr))
|| i > UINT_MAX)
return EKV_VAL_PARSE;
*val = i;
return EKV_SUCCESS;
Expand All @@ -51,12 +54,15 @@ int keyval_parse_int (const char *s, const char *key, int *val)
{
const char *cp = parse_val (s, key);
char *endptr;
int i;
long i;
if (!cp)
return EKV_NOKEY;
errno = 0;
i = strtol (cp, &endptr, 10);
if (errno != 0 || (*endptr && !isspace (*endptr)))
if (errno != 0
|| (*endptr && !isspace (*endptr))
|| i > INT_MAX
|| i < INT_MIN)
return EKV_VAL_PARSE;
*val = i;
return EKV_SUCCESS;
Expand Down
76 changes: 45 additions & 31 deletions src/common/libpmi/pmi.c
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,15 @@
return result;
}

int PMI_Abort (int exit_code, const char error_msg[])
int PMI_Abort (int exit_code, const char *error_msg)
{
/* pmi_simple_client_abort() only returns on error, in which case
* we fall back to printing the message on stderr and call exit()
* (return code not checked because we don't do anything with it)
*/
(void) pmi_simple_client_abort (pmi_global_ctx, exit_code, error_msg);
fprintf (stderr, "PMI_Abort: (%d) %s\n",
fprintf (stderr,

Check warning on line 98 in src/common/libpmi/pmi.c

View check run for this annotation

Codecov / codecov/patch

src/common/libpmi/pmi.c#L98

Added line #L98 was not covered by tests
"PMI_Abort: (%d) %s\n",
pmi_global_ctx ? pmi_global_ctx->rank : -1,
error_msg);
exit (exit_code);
Expand Down Expand Up @@ -133,7 +134,7 @@
return pmi_simple_client_get_appnum (pmi_global_ctx, appnum);
}

int PMI_KVS_Get_my_name (char kvsname[], int length)
int PMI_KVS_Get_my_name (char *kvsname, int length)
{
return pmi_simple_client_kvs_get_my_name (pmi_global_ctx,
kvsname,
Expand Down Expand Up @@ -170,13 +171,15 @@
return PMI_SUCCESS;
}

int PMI_KVS_Put (const char kvsname[], const char key[], const char value[])
int PMI_KVS_Put (const char *kvsname, const char *key, const char *value)
{
return pmi_simple_client_kvs_put (pmi_global_ctx, kvsname, key, value);
}

int PMI_KVS_Get (const char kvsname[], const char key[],
char value[], int length)
int PMI_KVS_Get (const char *kvsname,
const char *key,
char *value,
int length)
{
return pmi_simple_client_kvs_get (pmi_global_ctx,
kvsname,
Expand All @@ -185,7 +188,7 @@
length);
}

int PMI_KVS_Commit (const char kvsname[])
int PMI_KVS_Commit (const char *kvsname)
{
if (!pmi_global_ctx || !pmi_global_ctx->initialized)
return PMI_ERR_INIT;
Expand All @@ -199,38 +202,38 @@
return pmi_simple_client_barrier (pmi_global_ctx);
}

int PMI_Publish_name (const char service_name[], const char port[])
int PMI_Publish_name (const char *service_name, const char *port)
{
return PMI_FAIL;
}

int PMI_Unpublish_name (const char service_name[])
int PMI_Unpublish_name (const char *service_name)
{
return PMI_FAIL;
}

int PMI_Lookup_name (const char service_name[], char port[])
int PMI_Lookup_name (const char *service_name, char *port)
{
return PMI_FAIL;
}

int PMI_Spawn_multiple(int count,
const char * cmds[],
const char ** argvs[],
const int maxprocs[],
const int info_keyval_sizesp[],
const PMI_keyval_t * info_keyval_vectors[],
const char **cmds,
const char **argvs[],
const int *maxprocs,
const int *info_keyval_sizesp,
const PMI_keyval_t **info_keyval_vectors,
int preput_keyval_size,
const PMI_keyval_t preput_keyval_vector[],
int errors[])
const PMI_keyval_t *preput_keyval_vector,
int *errors)
{
return PMI_FAIL;
}

/* Old API funcs - signatures needed for ABI compliance.
*/

int PMI_Get_clique_ranks (int ranks[], int length)
int PMI_Get_clique_ranks (int *ranks, int length)
{
return pmi_simple_client_get_clique_ranks (pmi_global_ctx, ranks, length);
}
Expand All @@ -245,51 +248,62 @@
return PMI_KVS_Get_name_length_max (length);
}

int PMI_Get_id (char kvsname[], int length)
int PMI_Get_id (char *kvsname, int length)
{
return PMI_KVS_Get_my_name (kvsname, length);
}

int PMI_Get_kvs_domain_id (char kvsname[], int length)
int PMI_Get_kvs_domain_id (char *kvsname, int length)
{
return PMI_KVS_Get_my_name (kvsname, length);
}

int PMI_KVS_Create (char kvsname[], int length)
int PMI_KVS_Create (char *kvsname, int length)
{
return PMI_FAIL;
}

int PMI_KVS_Destroy (const char kvsname[])
int PMI_KVS_Destroy (const char *kvsname)
{
return PMI_FAIL;
}

int PMI_KVS_Iter_first (const char kvsname[], char key[], int key_len,
char val[], int val_len)
int PMI_KVS_Iter_first (const char *kvsname,
char *key,
int key_len,
char *val,
int val_len)
{
return PMI_FAIL;
}

int PMI_KVS_Iter_next (const char kvsname[], char key[], int key_len,
char val[], int val_len)
int PMI_KVS_Iter_next (const char *kvsname,
char *key,
int key_len,
char *val,
int val_len)
{
return PMI_FAIL;
}

int PMI_Parse_option (int num_args, char *args[], int *num_parsed,
PMI_keyval_t **keyvalp, int *size)
int PMI_Parse_option (int num_args,
char **args,
int *num_parsed,
PMI_keyval_t **keyvalp,
int *size)
{
return PMI_FAIL;
}

int PMI_Args_to_keyval (int *argcp, char *((*argvp)[]),
PMI_keyval_t **keyvalp, int *size)
int PMI_Args_to_keyval (int *argcp,
char *((*argvp)[]),
PMI_keyval_t **keyvalp,
int *size)
{
return PMI_FAIL;
}

int PMI_Free_keyvals (PMI_keyval_t keyvalp[], int size)
int PMI_Free_keyvals (PMI_keyval_t *keyvalp, int size)
{
return PMI_FAIL;
}
Expand Down