Skip to content

gh-127604: Replace dprintf() with _Py_write_noraise()#132854

Merged
vstinner merged 2 commits into
python:mainfrom
vstinner:puts
Apr 23, 2025
Merged

gh-127604: Replace dprintf() with _Py_write_noraise()#132854
vstinner merged 2 commits into
python:mainfrom
vstinner:puts

Conversation

@vstinner

@vstinner vstinner commented Apr 23, 2025

Copy link
Copy Markdown
Member

@vstinner

Copy link
Copy Markdown
Member Author

cc @ZeroIntensity

@ZeroIntensity ZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I wasn't aware of _Py_DumpHexadecimal, this works a lot better than the mess of dup and file APIs in #132800. Thanks!

@vstinner vstinner enabled auto-merge (squash) April 23, 2025 19:47
@vstinner vstinner merged commit 402dba2 into python:main Apr 23, 2025
@vstinner vstinner deleted the puts branch April 23, 2025 20:02
@tomasr8

tomasr8 commented Apr 24, 2025

Copy link
Copy Markdown
Member

This introduced a compiler warning:

Python/traceback.c:883:1: warning: ‘dump_pointer’ defined but not used [-Wunused-function]
  883 | dump_pointer(int fd, void *ptr)
      | ^~~~~~~~~~~~

Maybe the function should also be behind an #ifdef?

@vstinner

Copy link
Copy Markdown
Member Author

Python/traceback.c:883:1: warning: ‘dump_pointer’ defined but not used [-Wunused-function]

I wrote #132897 to fix the warning. On which machine/OS did you see the warning?

@vstinner

Copy link
Copy Markdown
Member Author

Aha, now I can see the warning on the WASI build. The configure script says:

checking for backtrace... (cached) no
checking for backtrace_symbols... (cached) no

@tomasr8

tomasr8 commented Apr 25, 2025

Copy link
Copy Markdown
Member

On which machine/OS did you see the warning?

I saw it on Ubuntu 24.04

@vstinner

Copy link
Copy Markdown
Member Author

I saw it on Ubuntu 24.04

Ok, good to know. I suppose that you miss backtrace_symbol() function. Likely a missing dependency. On Fedora, the /usr/include/execinfo.h header is provided by the glibc-devel package.

Anyway, the warning was fixed by c292f7f. Thanks for the report.

@tomasr8

tomasr8 commented Apr 25, 2025

Copy link
Copy Markdown
Member

Interesting, I do have the /usr/include/execinfo.h header, but the backtrace_symbol function is missing:

extern int backtrace (void **__array, int __size) __nonnull ((1));

extern char **backtrace_symbols (void *const *__array, int __size)
     __THROW __nonnull ((1));

extern void backtrace_symbols_fd (void *const *__array, int __size, int __fd)
     __THROW __nonnull ((1));

@vstinner

vstinner commented Apr 25, 2025

Copy link
Copy Markdown
Member Author

configure.ac looks for these functions:

$ grep backtrace configure.ac -B1
AC_CHECK_HEADERS([execinfo.h link.h dlfcn.h],
                 [AC_CHECK_FUNCS(backtrace backtrace_symbols dladdr1)])

Sorry, it's backtrace_symbols() with S :-)

@ZeroIntensity: By the way, backtrace_symbols test can now removed from configure.ac, it's no longer used, right?

@ZeroIntensity

Copy link
Copy Markdown
Member

Yup, but I don't know of a system that provides backtrace but not backtrace_symbols.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants