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

Error buffer always empty #532

Open
trivalik opened this issue Sep 13, 2024 · 6 comments · May be fixed by #533
Open

Error buffer always empty #532

trivalik opened this issue Sep 13, 2024 · 6 comments · May be fixed by #533

Comments

@trivalik
Copy link
Contributor

trivalik commented Sep 13, 2024

I tested only on windows with npcap 1.79 and found that the error buffer is always empty returned. This could be easily tested by:

var errorBuffer = new StringBuilder(Pcap.PCAP_ERRBUF_SIZE);
const uint NOT_VALID_ENCODING = 5;
var res = pcap_init(NOT_VALID_ENCODING, errorBuffer);

Does it make sense to allocated more memory than 256 for error buffer in the custom marshaler ?

@kayoub5
Copy link
Collaborator

kayoub5 commented Sep 13, 2024

The fact that a specific version of pcap in a specific OS returns an empty buffer, does not mean future versions will do so as well.

The allocation size of 256, is part of the libpcap API, not respecting it, could lead to memory corruption.

@trivalik
Copy link
Contributor Author

trivalik commented Sep 13, 2024

I can see in the custom marshaler that there is a proper string. I mean "more" than 256, 256 would be enough. I just ask, because it could be that other pcap implementations need that.

@kayoub5
Copy link
Collaborator

kayoub5 commented Sep 13, 2024

I assume by custom marshaler, you are referring to PcapStringMarshaler, it only exists to handle the fact that different operating systems could use different strings encodings, this has no effect on size, except adding 1 byte for '\0' string termination.

there are only 3 pcap implementations I am aware of, libpcap, WinPcap and Npcap and all of them comply with libpcap API, what other implementations are you referring to?

I don't really get where you see that more than 256 bytes are being allocated.

@trivalik
Copy link
Contributor Author

trivalik commented Sep 13, 2024

This too much allocation happens just for StringBuilder. In the customer marshaler in MarshalManagedToNative is StringEncoding.GetMaxByteCount(builder.Capacity) called, which returns 771. This number + 1 is then allocated.

The important fact is that the marshaler is not able to return something for StringBuilder. I tried out with other marshaling approaches but was not able to get values, except for values types like char[] or byte[].

@kayoub5
Copy link
Collaborator

kayoub5 commented Sep 15, 2024

Ok, I get that the custom marshaler is over allocating, but that should not have performance or functionality side effects, a PR to fix that is welcome.

The important fact is that the marshaler is not able to return something for StringBuilder. I tried out with other marshaling approaches but was not able to get values, except for values types like char[] or byte[]

I don't get what you are trying to do here, or what problem you are running into.

@kayoub5 kayoub5 linked a pull request Sep 15, 2024 that will close this issue
@kayoub5
Copy link
Collaborator

kayoub5 commented Sep 15, 2024

@trivalik Ok, I was able to reproduce the problem, the StringBuilder not getting the data back had absolutely nothing with the allocated memory size.

It seems to be a limitation of ICustomMarshaler API, silently hiding the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants