FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-127604: Replace dprintf() with _Py_write_noraise() by vstinner · Pull Request #132854 · python/cpython · GitHub

/ cpython Public

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 commented Apr 23, 2025
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

Copy link
Copy Markdown
Member Author

cc @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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

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

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?

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?

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 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

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 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 commented Apr 25, 2025
edited
Loading

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?

Copy link
Copy Markdown
Member

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL