GeezerVison

Ok, the first part with constant sized elements is done. Now, more complex ones.

Looks like we have PyOS_snprintf instead of just snprintf 🤔

Almost done!

There are a couple left, but I don't quite understand the code and I will highly appreciate help / advice on these cases:

• cpython/Modules/_ctypes/_ctypes.c

  Lines 2642 to 2651 in 5839575

  	cp += sprintf(cp, "%x", Py_SAFE_DOWNCAST(index, Py_ssize_t, int));
  	while (target->b_base) {
  	bytes_left = sizeof(string) - (cp - string) - 1;
  	/* Hex format needs 2 characters per byte */
  	if (bytes_left < sizeof(Py_ssize_t) * 2) {
  	PyErr_SetString(PyExc_ValueError,
  	"ctypes object structure too deep");
  	return NULL;
  	}
  	cp += sprintf(cp, ":%x", Py_SAFE_DOWNCAST(target->b_index, Py_ssize_t, int));

• cpython/Objects/unicodeobject.c

  Line 736 in 5839575

  size = sprintf(str, "&#%d;", PyUnicode_READ(kind, data, i));

• cpython/Parser/string_parser.c

  Line 121 in 5839575

  sprintf(p, "\\U%08x", chr);

• cpython/Python/pystrtod.c

  Line 1242 in 5839575

  exp_len = sprintf(p, "%+.02d", exp);

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 9bc0864 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit c7dd348 🤖

If you want to schedule another build, you need to add the :hammer: test-with-buildbots label again.

@Yhg1s @gvanrossum it is ready to be reviewed :)

I've made the requested changes, thanks for the suggestions 👍

@Yhg1s can you please review again? :)

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 8a95f08 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

I've merged it with the latest main, new round of buildbot tests are required to make sure that all still works correctly.

Does anyone want to take a look once again, please? 😉

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 3e8e61f 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit d82e315 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

In the past, I saw people introducing bugs and security vulnerabilities by attempting to make linters happy about getting rid of "dangerous functions".

The worst example that I recall:

• https://bugs.php.net/bug.php?id=55439
• Change introducing the security issue: php/php-src@7eb5bbb : "Fix more signed 1-bit bitfield, and let's use strlcpy/strlcat instead for these static string copies". Replace "unsafe" strncpy()/strncat() with "safe" strlcpy()/strncat().
• Fix : php/php-src@4f98090 (as Subversion commit)

Commit message of the fix:

commit 4f980905a0bff94807ea07cb897c0e4cd4e6b83f
Author: Stanislav Malyshev <[email protected]>
Date:   Fri Aug 19 22:49:18 2011 +0000

    Unbreak crypt() (fix bug #55439)
    # If you want to remove static analyser messages, be my guest,
    # but please run unit tests after

Well, at that time, PHP had no automated test suite, and running tests showed the regression. But well, it's surprising how using "safer" function can introduce a major security vulnerability.

The bug allowed to bypass any kind of login page (implemented with crypt()) with any password...

I wanted to close this PR anyway, I think that we should do it on a per-usage approach. Easier to review, easier to merge.