11
29
Fork
You've already forked ipmitool
33

lan: Clean compile time warnings from LAN authentication #22

Merged
AlexanderAmelkin merged 3 commits from Howitzer105mm/ipmitool:bugfix/21-fix-auth-compile-issues into master 2024年03月26日 12:38:41 +01:00
Contributor
Copy link

The MD2 and MD5 encryption schemes are deprecated. The compile time
switches, assigned from config.h, do not change which section of code
is compiled. The #ifdef is always seen as TRUE.

It is preferred to use #if to select the code to be compiled. Now
changing the HAVE_CRYPTO_MDx entries in the config.h file perform the
correct code compilation.

There are also two unused include files, which have been removed.
Resolves ipmitool/ipmitool/#21
Signed-off-by: Johnathan Mantey johnathanx.mantey@intel.com

The MD2 and MD5 encryption schemes are deprecated. The compile time switches, assigned from config.h, do not change which section of code is compiled. The #ifdef is always seen as TRUE. It is preferred to use #if to select the code to be compiled. Now changing the HAVE_CRYPTO_MDx entries in the config.h file perform the correct code compilation. There are also two unused include files, which have been removed. Resolves ipmitool/ipmitool/#21 Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
Howitzer105mm force-pushed bugfix/21-fix-auth-compile-issues from 53114d17e1 to ccec5529f9 2024年02月09日 20:59:20 +01:00 Compare
vmauery approved these changes 2024年02月10日 00:24:23 +01:00
Dismissed
AlexanderAmelkin left a comment
Copy link

Also, please update the commit log summary in accordance with the project rules:
https://codeberg.org/ipmitool/ipmitool/wiki/How-to-contribute#commit-preparation-rules

That is, make it lan: Fix compile time warnings in auth module

Also, please update the commit log summary in accordance with the project rules: https://codeberg.org/ipmitool/ipmitool/wiki/How-to-contribute#commit-preparation-rules That is, make it `lan: Fix compile time warnings in auth module`
@ -65,3 +63,3 @@
uint8_t * ipmi_auth_md5(struct ipmi_session * s, uint8_t * data, int data_len)
{
#ifdef HAVE_CRYPTO_MD5
#if HAVE_CRYPTO_MD5

Sorry, I don't quite get why you think that "the compile time switches, assigned from config.h, do not change which section of code is compiled" and that "the #ifdef is always seen as TRUE". Can you elaborate? Please keep in mind that config.h is not for manual modification, it is automatically generated by configure. When a feature is present, the macro is set to 1. When the feature is not present, the macro is NOT defined at all. Hence, the current method should work fine always.

I wouldn't change those #ifdefs unless absolutely necessary. What is the exact problem we're solving here?

Sorry, I don't quite get why you think that "the compile time switches, assigned from config.h, do not change which section of code is compiled" and that "the #ifdef is always seen as TRUE". Can you elaborate? Please keep in mind that `config.h` is not for manual modification, it is automatically generated by `configure`. When a feature is present, the macro is set to 1. When the feature is not present, the macro is NOT defined at all. Hence, the current method should work fine always. I wouldn't change those `#ifdef`s unless absolutely necessary. What is the exact problem we're solving here?
@ -212,2 +210,3 @@
md5_init(&state);
md5_append(&state, (const md5_byte_t *)s->authcode, strlen(s->authcode));
md5_append(&state, (const md5_byte_t *)s->authcode,
strlen((const char *)s->authcode));

Could you please align strlen to &state ? As in "tabs for indentation, spaces for alignment". I see that now this line indented with 3 tabs. I would expect a single tab and a bunch of spaces after it.

Could you please align `strlen` to `&state` ? As in "tabs for indentation, spaces for alignment". I see that now this line indented with 3 tabs. I would expect a single tab and a bunch of spaces after it.
Author
Contributor
Copy link

I do not see an issue.
I see two tabs followed by three SPC chars.

I do not see an issue. I see two tabs followed by three SPC chars.
First-time contributor
Copy link

there should be a single tab to align with the start of md5_append and then spaces to align up to the start of &state

there should be a single tab to align with the start of md5_append and then spaces to align up to the start of &state
Author
Contributor
Copy link

Not behavior I expect.
Updated to comply.

Not behavior I expect. Updated to comply.

Thank you. Here you can find some good rationale for what I was asking for:
https://dmitryfrank.com/articles/indent_with_tabs_align_with_spaces

Thank you. Here you can find some good rationale for what I was asking for: https://dmitryfrank.com/articles/indent_with_tabs_align_with_spaces
The MD2 and MD5 encryption schemes are deprecated. The compile time
switches, assigned from config.h, do not change which section of code
is compiled. The #ifdef is always seen as TRUE.
It is preferred to use #if to select the code to be compiled. Now
changing the HAVE_CRYPTO_MDx entries in the config.h file perform the
correct code compilation.
There are also two unused include files, which have been removed.
Resolves ipmitool/ipmitool/#21
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Howitzer105mm changed title from (削除) Clean compile time warnings from LAN authentication (削除ここまで) to lan: Clean compile time warnings from LAN authentication 2024年02月21日 22:09:15 +01:00 
The MD2 and MD5 encryption schemes are deprecated. The compile time
switches, assigned from config.h, do not change which section of code
is compiled. The #ifdef is always seen as TRUE.
It is preferred to use #if to select the code to be compiled. Now
changing the HAVE_CRYPTO_MDx entries in the config.h file perform the
correct code compilation.
There are also two unused include files, which have been removed.
Resolves ipmitool/ipmitool/#21
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
Sign in to join this conversation.
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
IPMITool/ipmitool!22
Reference in a new issue
IPMITool/ipmitool
No description provided.
Delete branch "Howitzer105mm/ipmitool:bugfix/21-fix-auth-compile-issues"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?