Skip to content

Commit

Permalink
Use void* free_me in new functions that use it
Browse files Browse the repository at this point in the history
Two new functions were recently added that have a *free_me parameter.
As Tony Cook pointed out in Perl#22812, these would better have been void*
instead of U8*.  This commit changes to use void*

One function in pp.c was using this parameter as a U8* and using its
contents.  But this need goes away with the new bytes_to_utf8_free_me()
function, which eliminates the kludge pp.c used to make sure the new
memory got freed.
  • Loading branch information
khwilliamson committed Jan 12, 2025
1 parent 6806ac9 commit 76974c9
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 24 deletions.
6 changes: 3 additions & 3 deletions embed.fnc
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ Admp |U8 * |bytes_to_utf8 |NN const U8 *s \
Adp |U8 * |bytes_to_utf8_free_me \
|NN const U8 *s \
|NN STRLEN *lenp \
|NULLOK const U8 **free_me
|NULLOK void **free_me
AOdp |SSize_t|call_argv |NN const char *sub_name \
|I32 flags \
|NN char **argv
Expand Down Expand Up @@ -3711,12 +3711,12 @@ Adpx |U8 * |utf8_to_bytes |NN U8 *s \
|NN STRLEN *lenp
Cp |bool |utf8_to_bytes_ |NN U8 **s_ptr \
|NN STRLEN *lenp \
|NN U8 **free_me \
|NN void **free_me \
|Perl_utf8_to_bytes_arg result_as
Admp |bool |utf8_to_bytes_new_pv \
|NN U8 const **s_ptr \
|NN STRLEN *lenp \
|NN U8 *free_me
|NN void **free_me
Admp |bool |utf8_to_bytes_overwrite \
|NN U8 **s_ptr \
|NN STRLEN *lenp
Expand Down
8 changes: 4 additions & 4 deletions hv.c
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ S_hv_delete_common(pTHX_ HV *hv, SV *keysv, const char *key, STRLEN klen,

if (is_utf8 && !(k_flags & HVhek_KEYCANONICAL)) {
const char * const keysave = key;
U8 * free_me = NULL;
void * free_me = NULL;

if (! utf8_to_bytes_new_pv(&key, &klen, &free_me)) {
k_flags |= HVhek_UTF8;
Expand Down Expand Up @@ -3270,7 +3270,7 @@ S_unshare_hek_or_pvn(pTHX_ const HEK *hek, const char *str, I32 len, U32 hash)
} else if (len < 0) {
STRLEN tmplen = -len;
/* See the note in hv_fetch(). --jhi */
U8 * free_str = NULL;
void * free_str = NULL;
if (! utf8_to_bytes_new_pv(&str, &tmplen, &free_str)) {
k_flags = HVhek_UTF8;
}
Expand Down Expand Up @@ -3687,7 +3687,7 @@ Perl_refcounted_he_fetch_pvn(pTHX_ const struct refcounted_he *chain,
PERL_ARGS_ASSERT_REFCOUNTED_HE_FETCH_PVN;

U8 utf8_flag;
U8 * free_me = NULL;
void * free_me = NULL;

if (flags & ~(REFCOUNTED_HE_KEY_UTF8|REFCOUNTED_HE_EXISTS))
Perl_croak(aTHX_ "panic: refcounted_he_fetch_pvn bad flags %" UVxf,
Expand Down Expand Up @@ -3821,7 +3821,7 @@ Perl_refcounted_he_new_pvn(pTHX_ struct refcounted_he *parent,
char hekflags;
STRLEN key_offset = 1;
struct refcounted_he *he;
U8 * free_me = NULL;
void * free_me = NULL;

if (!value || value == &PL_sv_placeholder) {
value_type = HVrhek_delete;
Expand Down
15 changes: 7 additions & 8 deletions pp.c
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ S_do_chomp(pTHX_ SV *retval, SV *sv, bool chomping)
s = SvPV(sv, len);
if (chomping) {
if (s && len) {
U8 *temp_buffer = NULL;
void *free_me = NULL;
s += --len;
if (RsPARA(PL_rs)) {
if (*s != '\n')
Expand All @@ -817,18 +817,17 @@ S_do_chomp(pTHX_ SV *retval, SV *sv, bool chomping)
/* Assumption is that rs is shorter than the scalar. */
if (SvUTF8(PL_rs)) {
/* RS is utf8, scalar is 8 bit. */
if (! utf8_to_bytes_new_pv(&rsptr, &rslen,
&temp_buffer))
{
if (! utf8_to_bytes_new_pv(&rsptr, &rslen, &free_me)) {
/* Cannot downgrade, therefore cannot possibly
* match. */
goto nope_free_nothing;
}
}
else {
/* RS is 8 bit, scalar is utf8. */
temp_buffer = bytes_to_utf8((U8*)rsptr, &rslen);
rsptr = (char *) temp_buffer;
rsptr = (char *) bytes_to_utf8_free_me((U8*) rsptr,
&rslen,
&free_me);
}
}
if (rslen == 1) {
Expand All @@ -853,7 +852,7 @@ S_do_chomp(pTHX_ SV *retval, SV *sv, bool chomping)
SvSETMAGIC(sv);

nope_free_all:
Safefree(temp_buffer);
Safefree(free_me);
nope_free_nothing: ;
}
} else {
Expand Down Expand Up @@ -3819,7 +3818,7 @@ PP(pp_index)
if (little_utf8) {
/* Well, maybe instead we might be able to downgrade the small
string? */
U8 * free_little_p = NULL;
void * free_little_p = NULL;
if (utf8_to_bytes_new_pv(&little_p, &llen, &free_little_p)) {
little_utf8 = false;

Expand Down
6 changes: 3 additions & 3 deletions proto.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions utf8.c
Original file line number Diff line number Diff line change
Expand Up @@ -2958,7 +2958,7 @@ New code should use the first three functions listed above.
*/

bool
Perl_utf8_to_bytes_(pTHX_ U8 **s_ptr, STRLEN *lenp, U8 ** free_me,
Perl_utf8_to_bytes_(pTHX_ U8 **s_ptr, STRLEN *lenp, void ** free_me,
Perl_utf8_to_bytes_arg result_as)
{
PERL_ARGS_ASSERT_UTF8_TO_BYTES_;
Expand Down Expand Up @@ -3237,7 +3237,7 @@ Perl_bytes_from_utf8(pTHX_ const U8 *s, STRLEN *lenp, bool *is_utf8p)
PERL_ARGS_ASSERT_BYTES_FROM_UTF8;

if (*is_utf8p) {
U8 * new_memory = NULL;
void * new_memory = NULL;
if (utf8_to_bytes_new_pv(&s, lenp, &new_memory)) {
*is_utf8p = false;

Expand Down Expand Up @@ -3287,7 +3287,7 @@ But when it is a non-NULL pointer, C<bytes_to_utf8_free_me> stores into it
either NULL if no memory was allocated; or a pointer to that new memory. This
allows the following convenient paradigm:
U8 * free_me;
void * free_me;
U8 converted = bytes_to_utf8_free_me(string, &len, &free_me);
...
Expand All @@ -3297,6 +3297,8 @@ allows the following convenient paradigm:
You don't have to know if memory was allocated or not. Just call C<Safefree>
unconditionally. C<free_me> will contain a suitable value to pass to
C<Safefree> for it to do the right thing, regardless.
Your design is likely flawed if you find yourself using C<free_me> for anything
other than passing to C<Safefree>.
Upon return, the number of variants in the string can be computed by
having saved the value of C<*lenp> before the call, and subtracting it from the
Expand All @@ -3310,7 +3312,7 @@ EBCDIC), see L</sv_recode_to_utf8>().

U8*
Perl_bytes_to_utf8_free_me(pTHX_ const U8 *s, Size_t *lenp,
const U8 ** free_me_ptr)
void ** free_me_ptr)
{
PERL_ARGS_ASSERT_BYTES_TO_UTF8_FREE_ME;
PERL_UNUSED_CONTEXT;
Expand Down
4 changes: 2 additions & 2 deletions utf8.h
Original file line number Diff line number Diff line change
Expand Up @@ -1341,13 +1341,13 @@ typedef enum {
* there is a NN assertion for it. It causes that to pass but to still
* segfault if wrongly gets used */
#define Perl_utf8_to_bytes_overwrite(mTHX, s, l) \
Perl_utf8_to_bytes_(aTHX_ s, l, INT2PTR(U8 **, 1), \
Perl_utf8_to_bytes_(aTHX_ s, l, INT2PTR(void **, 1), \
PL_utf8_to_bytes_overwrite)
#define Perl_utf8_to_bytes_new_pv(mTHX, s, l, f) \
Perl_utf8_to_bytes_(aTHX_ (U8 **) s, l, f, \
PL_utf8_to_bytes_new_memory)
#define Perl_utf8_to_bytes_temp_pv(mTHX, s, l) \
Perl_utf8_to_bytes_(aTHX_ (U8 **) s, l, INT2PTR(U8 **, 1), \
Perl_utf8_to_bytes_(aTHX_ (U8 **) s, l, INT2PTR(void **, 1), \
PL_utf8_to_bytes_use_temporary)

/* Do not use; should be deprecated. Use isUTF8_CHAR() instead; this is
Expand Down

0 comments on commit 76974c9

Please sign in to comment.