-
Notifications
You must be signed in to change notification settings - Fork 811
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
Ada wrapper incorrectly sets verification method for remote peers #7461
Comments
@joakim-strandberg can you please review this bug report? |
@dalybrown good catch! The Ada code should be a direct translation of the C examples and that translation does not look correct. My bad, thanks for finding it!! The best way to fix it? One idea is to rename the function 'function "&" (Left, Right : Mode_Type) return Mode_Type;' to 'function "|" (Left, Right : Mode_Type) return Mode_Type;' and instead of "and" in the implementation "or" should be used instead. It will make the Ada code look more like the C implementation when using the wolfSSL library. What do you think? |
My preference would be to use the existing Ada syntax for logical operators: In general, I like things to not look like C haha... but that's my opinion. |
p.s., there is another issue too with examples: the |
p.p.s, I'm happy to fix both issues here. Let me know. |
Hi @dalybrown , yes feel free to put up a PR fixing both issues. You are right about needing |
It's reasonable to use the "or" operator instead of the "|" operator. You can choose either way @dalybrown. I do note that we are in a gray zone. The Ada language uses both the keyword "or" and "|" to list alternatives. In if-statements the "or" keyword is used, for example "if A or B then" but in case-statement over an enumeration type the "|" is used for example "when Red | Green | Blue =>". In the C version of the wolfSSL library it is known that the Method_Type is an unsigned 32-bit number and one uses bit-wise operations to turn on specific flags. In the Ada version of the wolfSSL library the Method_Type is a private type indicating to the user that the exact type of a Method_Type instance is unknown. It's a fundamental idea in Ada to hide exact representations from users as much as possible to limit potential errors. Classical example is the Integer type where one does not have access to the representation. The major argument for that is the code should be cross-platform and negative numbers may be represented in different ways depending on the hardware. So, in an Ada binding the Method_Type should be private, so it is not known what Method_Type is, but then using the "or" operator to list alternatives implicates that the Method_Type is actually a modular type defined in the Interfaces package and the "or" operator defined there is used under the hood. Which is true, but then again I am thinking is it possible to hide this fact and still have an Ada binding that is easy to use and understand? Just something to keep in mind. I don't think we should spend time on this. It's good to be pragmatic, pushing things forward, everything doesn't have to be perfect. |
That's a good point. WolfSSL happens to use a logical operator here. There is probably a more idiomatic Ada way of specifying this than having clients do any logical operations. I'll think about it for a bit and circle back with a few options. One way would be to track the state of the mode that is set, and have a subprogram that you continually set the flags. Under the hood it will perform the logical or operation but that's not exposed to the client.
and then in the body you could just the operator:
You can then just call it for every flag you want to set and then call the regular procedure. There are probably many other options. Thoughts? |
Maybe better names would be:
Or something... |
Hi @dalybrown , will you be able to put up a PR to fix this mask and the netdb_h? |
Definitely! I was hoping to hear back from @joakim-strandberg on my suggestions prior to pushing any changes. |
Hi @dalybrown , Is this issue resolved? Let me know if you need anything. Thanks , |
Ah I didn't get back to this yet - sorry! It's been a busy summer. I haven't forgotten - will get back to this hopefully in September. |
I think there is a bug in the logic for setting the verification method in the Ada wrapper here.
The documentation suggests it should be a logical OR operator here.
However, it performs an AND operator here.
Unless the documentation is incorrect! But the C examples suggest it is a logical OR here.
The text was updated successfully, but these errors were encountered: