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

[feature] Make default typemap type-mismatch errors give the passed-in SV’s information #18461

Open
FGasper opened this issue Jan 8, 2021 · 3 comments

Comments

@FGasper
Copy link
Contributor

FGasper commented Jan 8, 2021

I’m chasing one of these:

    (in cleanup) DNS::Unbound::_destroy_context: ctx is not of type struct ub_ctxPtr at /usr/local/cpanel/3rdparty/perl/532/lib/perl5/cpanel_lib/x86_64-linux-64int/DNS/Unbound.pm line 731 during global destruction.

Currently DNS::Unbound uses the default T_PTROBJ typemap. I’m about to push up a change to the module to use a custom typemap that will print this instead:

DNS::Unbound::_destroy_context: ctx (scalar: 123123) is not of type struct ub_ctxPtr at …

I thought I’d throw this up here to gauge interest in making this change to upstream. I’m including a provisional PR.

Thank you!

@tonycoz
Copy link
Contributor

tonycoz commented Jan 10, 2021

Looking at the commit in your repository:

const char* valstr = SvROK($arg) ? sv_2pvbyte_nolen($arg) : SvPV_nolen($arg);

why differentiate? (and why call the function directly?) Also, the object could be blessed into a non-ASCII package name,

Also this could just use the SVf macro:

Perl_croak_nocontext(\"%s: %s (%s" SVf ") is not of type %s\",

and supply $arg instead of valstr. This also handles unicode strings and stash names for blessed objects.

@jkeenan
Copy link
Contributor

jkeenan commented Feb 11, 2021

@FGasper, would you like to respond to @tonycoz's reply? Otherwise, it looks like we should close this feature request.

Thank you very much.
Jim Keenan

@jkeenan jkeenan added Closable? We might be able to close this ticket, but we need to check with the reporter and removed Needs Triage labels Feb 11, 2021
@FGasper
Copy link
Contributor Author

FGasper commented Feb 11, 2021

@jkeenan I updated the PR to incorporate @tonycoz’s suggestion a bit back. I didn’t update this thread though .. sorry. :(

@toddr toddr added awaiting review and removed Closable? We might be able to close this ticket, but we need to check with the reporter labels Apr 9, 2021
khwilliamson pushed a commit that referenced this issue May 21, 2021
Corion pushed a commit to Corion/perl5 that referenced this issue Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants