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

fix macos portability issues (third batch) #6473

Merged
merged 9 commits into from
Dec 4, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ AC_CHECK_HEADERS( \
inttypes.h \
link.h \
[sys/ucred.h] \
[sys/prctl.h] \
)

##
Expand Down Expand Up @@ -236,6 +237,7 @@ AC_REPLACE_FUNCS( \
strerrorname_np \
pipe2 \
mempcpy \
get_current_dir_name \
)
X_AC_CHECK_PTHREADS
X_AC_CHECK_COND_LIB(rt, clock_gettime)
Expand All @@ -244,6 +246,17 @@ X_AC_MALLOC
AC_CHECK_LIB(m, floor)
AC_SEARCH_LIBS(epoll, libepoll-shim)

AC_MSG_CHECKING([for pthread_setname_np with tid parameter])
AC_COMPILE_IFELSE(
[AC_LANG_PROGRAM(
[#include <pthread.h>],
[pthread_setname_np(pthread_self(), "example")])],
[AC_MSG_RESULT(yes)
AC_DEFINE(HAVE_PTHREAD_SETNAME_NP_WITH_TID, 1,
[Have pthread_setname_np() with tid])],
[AC_MSG_RESULT(no)]
)

AC_ARG_ENABLE([docs],
AS_HELP_STRING([--disable-docs], [disable building docs]))

Expand Down
8 changes: 6 additions & 2 deletions src/broker/broker.c
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
#include <signal.h>
#include <locale.h>
#include <inttypes.h>
#ifdef HAVE_SYS_PRCTL_H
#include <sys/prctl.h>
#endif
#include <sys/resource.h>
#include <fcntl.h>
#include <unistd.h>
Expand Down Expand Up @@ -51,6 +53,7 @@
#include "src/common/libutil/errno_safe.h"
#include "src/common/libutil/errprintf.h"
#include "src/common/libutil/intree.h"
#include "src/common/libutil/basename.h"
#include "src/common/librouter/subhash.h"
#include "src/common/libfluxutil/method.h"
#include "ccan/array_size/array_size.h"
Expand Down Expand Up @@ -665,9 +668,11 @@ static void init_attrs (attr_t *attrs, pid_t pid, struct flux_msg_cred *cred)

static void set_proctitle (uint32_t rank)
{
#ifdef PR_SET_NAME
static char proctitle[32];
snprintf (proctitle, sizeof (proctitle), "flux-broker-%"PRIu32, rank);
(void)prctl (PR_SET_NAME, proctitle, 0, 0, 0);
#endif
}

static bool is_interactive_shell (const char *argz, size_t argz_len)
Expand All @@ -685,8 +690,7 @@ static bool is_interactive_shell (const char *argz, size_t argz_len)
char *shell;
char *cmd = argz_next (argz, argz_len, NULL);
while ((shell = getusershell ())) {
if (streq (cmd, shell)
|| streq (cmd, basename (shell))) {
if (streq (cmd, shell) || streq (cmd, basename_simple (shell))) {
result = true;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/broker/log.c
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,7 @@ static void log_timestamp (FILE *fp,
|| strftime (datetime, sizeof (datetime), "%b %d %T", &tm) == 0
|| strftime (timezone, sizeof (timezone), "%Z", &tm) == 0)
fprintf (fp, "%s ", hdr->timestamp);
fprintf (fp, "%s.%06ld %s ", datetime, tv.tv_usec, timezone);
fprintf (fp, "%s.%06ld %s ", datetime, (long)tv.tv_usec, timezone);
}

/* Log a message to 'fp', if non-NULL.
Expand Down
24 changes: 11 additions & 13 deletions src/broker/module.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "src/common/libutil/log.h"
#include "src/common/libutil/errprintf.h"
#include "src/common/libutil/errno_safe.h"
#include "src/common/libutil/basename.h"
#include "src/common/librouter/subhash.h"
#include "ccan/str/str.h"

Expand Down Expand Up @@ -106,7 +107,11 @@
name_ptr = local_name;
}
// Set the name of each thread to its module name
#if HAVE_PTHREAD_SETNAME_NP_WITH_TID
(void) pthread_setname_np (pthread_self (), name_ptr);
#else // e.g. macos
(void) pthread_setname_np (name_ptr);
#endif
return (0);
}

Expand Down Expand Up @@ -285,23 +290,16 @@
p->poller_cb (p, p->poller_arg);
}

static char *module_name_from_path (const char *s)
static char *module_name_from_path (const char *path)
{
char *path, *name, *cpy;
char *name;
char *cp;

if (!(path = strdup (s))
|| !(name = basename (path)))
goto error;
name = basename_simple (path);
// if path ends in .so or .so.VERSION, strip it off
if ((cp = strstr (name, ".so")))
*cp = '\0';
if (!(cpy = strdup (name)))
goto error;
free (path);
return cpy;
error:
ERRNO_SAFE_WRAP (free, path);
return NULL;
return strndup (name, cp - name);
return strdup (name);

Check warning on line 302 in src/broker/module.c

View check run for this annotation

Codecov / codecov/patch

src/broker/module.c#L302

Added line #L302 was not covered by tests
}

module_t *module_create (flux_t *h,
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/builtin/dmesg.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ void print_iso_timestamp (struct dmesg_ctx *ctx, struct stdlog_header *hdr)
printf ("%s%s.%.6lu%s%s ",
dmesg_color (ctx, DMESG_COLOR_TIME),
buf,
tv.tv_usec,
(unsigned long)tv.tv_usec,
tz,
dmesg_color_reset (ctx));
}
Expand Down
6 changes: 5 additions & 1 deletion src/cmd/flux-exec.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,17 @@
#include <flux/core.h>
#include <flux/optparse.h>
#include <signal.h>
#ifndef HAVE_GET_CURRENT_DIR_NAME
#include "src/common/libmissing/get_current_dir_name.h"
#endif

#include "src/common/libczmqcontainers/czmq_containers.h"
#include "src/common/libutil/xzmalloc.h"
#include "src/common/libutil/monotime.h"
#include "src/common/libidset/idset.h"
#include "src/common/libeventlog/eventlog.h"
#include "src/common/libutil/log.h"
#include "src/common/libutil/basename.h"
#include "src/common/libsubprocess/fbuf.h"
#include "src/common/libsubprocess/fbuf_watcher.h"
#include "ccan/str/str.h"
Expand Down Expand Up @@ -447,7 +451,7 @@ static bool check_for_imp_run (int argc, char *argv[], const char **ppath)
/* If argv0 basename is flux-imp, then we'll likely have to use
* flux-imp kill to signal the resulting subprocesses
*/
if (streq (basename (argv[0]), "flux-imp")) {
if (streq (basename_simple (argv[0]), "flux-imp")) {
*ppath = argv[0];
return true;
}
Expand Down
11 changes: 8 additions & 3 deletions src/cmd/job/mpir.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,16 @@
#include <unistd.h>
#include <errno.h>
#include <signal.h>
#include <libgen.h>
#ifndef HAVE_GET_CURRENT_DIR_NAME
#include "src/common/libmissing/get_current_dir_name.h"
#endif

#include <flux/core.h>
#include <flux/optparse.h>

#include "src/common/libutil/log.h"
#include "src/common/libutil/basename.h"
#include "src/common/libjob/idf58.h"
#include "src/common/libdebugged/debugged.h"
#include "src/shell/mpir/proctable.h"
Expand Down Expand Up @@ -139,7 +144,7 @@ static void completion_cb (flux_subprocess_t *p)
int rank = flux_subprocess_rank (p);
int exitcode = flux_subprocess_exit_code (p);
int signum = flux_subprocess_signaled (p);
const char *prog = basename (MPIR_executable_path);
const char *prog = basename_simple (MPIR_executable_path);

if (signum > 0)
log_msg ("MPIR: rank %d: %s: %s", rank, prog, strsignal (signum));
Expand Down Expand Up @@ -183,7 +188,7 @@ static void output_cb (flux_subprocess_t *p, const char *stream)
{
const char *line;
int len = 0;
const char *prog = basename (MPIR_executable_path);
const char *prog = basename_simple (MPIR_executable_path);
int rank = flux_subprocess_rank (p);

len = flux_subprocess_read_trimmed_line (p, stream, &line);
Expand All @@ -196,7 +201,7 @@ static void output_cb (flux_subprocess_t *p, const char *stream)
static void state_cb (flux_subprocess_t *p, flux_subprocess_state_t state)
{
if (state == FLUX_SUBPROCESS_FAILED) {
const char *prog = basename (MPIR_executable_path);
const char *prog = basename_simple (MPIR_executable_path);
int rank = flux_subprocess_rank (p);
const char *errmsg = flux_subprocess_fail_error (p);
log_msg ("MPIR: rank %d: %s: %s", rank, prog, errmsg);
Expand Down
3 changes: 2 additions & 1 deletion src/common/libmissing/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -52,4 +52,5 @@ EXTRA_libmissing_la_SOURCES = \
strerrorname_np.h \
json_object_update_recursive.h \
pipe2.h \
mempcpy.h
mempcpy.h \
get_current_dir_name.h
33 changes: 33 additions & 0 deletions src/common/libmissing/get_current_dir_name.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/************************************************************\
* Copyright 2024 Lawrence Livermore National Security, LLC
* (c.f. AUTHORS, NOTICE.LLNS, COPYING)
*
* This file is part of the Flux resource manager framework.
* For details, see https://github.com/flux-framework.
*
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

#if HAVE_CONFIG_H
#include "config.h"
#endif
#include <limits.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>

#include "get_current_dir_name.h"

char *get_current_dir_name (void)
{
char buf[PATH_MAX + 1];
char *name;
char *cpy;

if (!(name = getcwd (buf, sizeof (buf)))
|| !(cpy = strdup (name)))
return NULL;
return cpy;
}

// vi:ts=4 sw=4 expandtab
16 changes: 16 additions & 0 deletions src/common/libmissing/get_current_dir_name.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/************************************************************\
* Copyright 2024 Lawrence Livermore National Security, LLC
* (c.f. AUTHORS, NOTICE.LLNS, COPYING)
*
* This file is part of the Flux resource manager framework.
* For details, see https://github.com/flux-framework.
*
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

#ifndef _GET_CURRENT_DIR_NAME_H
#define _GET_CURRENT_DIR_NAME_H 1

char *get_current_dir_name (void);

#endif // !_GET_CURRENT_DIR_NAME_H
9 changes: 3 additions & 6 deletions src/common/libsdexec/unit.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "src/common/libmissing/macros.h"
#include "src/common/libutil/errprintf.h"
#include "src/common/libutil/aux.h"
#include "src/common/libutil/basename.h"
#include "ccan/str/str.h"
#include "ccan/array_size/array_size.h"

Expand Down Expand Up @@ -75,12 +76,8 @@ int sdexec_unit_aux_set (struct unit *unit,

const char *sdexec_unit_name (struct unit *unit)
{
if (unit) {
const char *cp;
if ((cp = strrchr (unit->path, '/')))
return cp + 1;
return unit->path;
}
if (unit)
return basename_simple (unit->path);
return "internal error: unit is null";
}

Expand Down
3 changes: 2 additions & 1 deletion src/common/libtomlc99/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ test_cppflags = \

test_ldadd = \
$(top_builddir)/src/common/libtomlc99/libtomlc99.la \
$(top_builddir)/src/common/libtap/libtap.la
$(top_builddir)/src/common/libtap/libtap.la \
$(top_builddir)/src/common/libutil/libutil.la

toml_cat_SOURCES = toml_cat.c
toml_cat_LDADD = $(test_ldadd)
Expand Down
5 changes: 3 additions & 2 deletions src/common/libtomlc99/test/toml.c
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <locale.h>

#include "src/common/libtap/tap.h"
#include "src/common/libutil/basename.h"
#include "toml.h"

#define EX1 "\
Expand Down Expand Up @@ -243,7 +244,7 @@ void parse_bad_input (void)

for (i = 0; i < results.gl_pathc; i++) {
char errbuf[255];
char *name = basename (results.gl_pathv[i]);
char *name = basename_simple (results.gl_pathv[i]);
const char *reason;
bool blocklist = matchtab (name, bad_input_blocklist, &reason);

Expand Down Expand Up @@ -299,7 +300,7 @@ void parse_good_input (void)
for (i = 0; i < results.gl_pathc; i++) {
char errbuf[255];
ok (parse_good_file (results.gl_pathv[i], errbuf, 255) == true,
"%s: %s", basename (results.gl_pathv[i]), errbuf);
"%s: %s", basename_simple (results.gl_pathv[i]), errbuf);
}
globfree (&results);
}
Expand Down
2 changes: 2 additions & 0 deletions src/common/libutil/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ libutil_la_SOURCES = \
sigutil.c \
parse_size.h \
parse_size.c \
basename.h \
basename.c \
ansi_color.h

TESTS = test_sha1.t \
Expand Down
28 changes: 28 additions & 0 deletions src/common/libutil/basename.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/************************************************************\
* Copyright 2016 Lawrence Livermore National Security, LLC
* (c.f. AUTHORS, NOTICE.LLNS, COPYING)
*
* This file is part of the Flux resource manager framework.
* For details, see https://github.com/flux-framework.
*
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

#if HAVE_CONFIG_H
#include "config.h"
#endif
#include <string.h>

#include "basename.h"

/* This is what glibc basename(3) does more or less.
* https://github.com/lattera/glibc/blob/master/string/basename.c
*/

char *basename_simple (const char *path)
{
char *p = strrchr (path, '/');
return p ? p + 1 : (char *)path;
}
Comment on lines +22 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice. Does this undo the need for 92c2ece if all uses of basename were updated to basename_simple()?

I know these are just portability updates, but perhaps we can update the code in shell/doom.c that does the same thing to use basename_simple.c instead:

flux-core/src/shell/doom.c

Lines 100 to 106 in 11ad605

static const char *get_jobspec_command_arg0 (struct shell_doom *doom)
{
json_t *s = json_array_get (doom->shell->info->jobspec->command, 0);
const char *path = json_string_value (s);
const char *p = strrchr (path, '/');
return p ? p + 1 : path;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sure, we can do that. I was trying to have a light touch but this makes sense.


// vi:ts=4 sw=4 expandtab
18 changes: 18 additions & 0 deletions src/common/libutil/basename.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/************************************************************\
* Copyright 2024 Lawrence Livermore National Security, LLC
* (c.f. AUTHORS, NOTICE.LLNS, COPYING)
*
* This file is part of the Flux resource manager framework.
* For details, see https://github.com/flux-framework.
*
* SPDX-License-Identifier: LGPL-3.0
\************************************************************/

#ifndef _UTIL_BASENAME_H
#define _UTIL_BASENAME_H

char *basename_simple (const char *path);

#endif /* !_UTIL_BASENAME_H */

// vi:ts=4 sw=4 expandtab
Loading
Loading