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

fix in nvram definition #105

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

ADITI-MISHRA310
Copy link

Fix errors related to nvram functions in src/nvram.c while building the powerpc-utils package in Fedora (Mentioned below), this is due to old C syntax present in powerpc-utls package code base.
Fixing definition of regressed functions are able to solve the issue.

Issue that I was getting:

src/nvram.c: In function ‘dump_rtas_event_entry’:
src/nvram.c:932:18: error: too many arguments to function ‘parse_rtas_event’; expected 0, have 2
  932 |     rtas_event = parse_rtas_event(data, len);
      |                  ^~~~~~~~~~~~~~~~ ~~~~
src/nvram.c:938:5: error: too many arguments to function ‘rtas_print_event’; expected 0, have 3
  938 |     rtas_print_event(stdout, rtas_event, 0);
      |     ^~~~~~~~~~~~~~~~ ~~~~~~
src/nvram.c:940:5: error: too many arguments to function ‘cleanup_rtas_event’; expected 0, have 1
  940 |     cleanup_rtas_event(rtas_event);
      |     ^~~~~~~~~~~~~~~~~~ ~~~~~~~~~~
In file included from src/vcpustat.c:34:
./src/common/pseries_platform.h:21: warning: header guard ‘PLATFORM_H’ followed by ‘#define’ of a different macro [-Wheader-guard]
   21 | #ifndef PLATFORM_H
./src/common/pseries_platform.h:22: note: ‘PLARFORM_H’ is defined here; did you mean ‘PLATFORM_H’?
   22 | #define PLARFORM_H
make: *** [Makefile:1085: src/nvram.o] Error 1
make: *** Waiting for unfinished jobs....
In file included from src/activate_fw.c:70:
./src/common/pseries_platform.h:21: warning: header guard ‘PLATFORM_H’ followed by ‘#define’ of a different macro [-Wheader-guard]
   21 | #ifndef PLATFORM_H
./src/common/pseries_platform.h:22: note: ‘PLARFORM_H’ is defined here; did you mean ‘PLATFORM_H’?
   22 | #define PLARFORM_H
RPM build errors:
error: Bad exit status from /var/tmp/rpm-tmp.hpdalO (%build)

Copy link

@davemq davemq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks correct. Thanks.

@tyreld
Copy link
Member

tyreld commented Feb 25, 2025

This is failing to build with our CI which does use -Werror. The problem is that there is no struct rtas_event type definition. We could include the librtasevent.h header to pull it in, but since we never dereference the struct pointer in the nvram.c code we can work with just an incomplete type.

Further, if we are correcting the function parameter lists we should also update the return types to match as well.

@@ -903,9 +903,9 @@ dump_rtas_event_entry(char *data, int len)
{
void *rtas_event;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be struct rtas_event *rtas_event

void *(*parse_rtas_event)();
void (*rtas_print_event)();
void (*cleanup_rtas_event)();
void *(*parse_rtas_event)(char*, int);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parse_rtas_event returns a struct rtas_event *

void (*rtas_print_event)();
void (*cleanup_rtas_event)();
void *(*parse_rtas_event)(char*, int);
void (*rtas_print_event)(FILE *, struct rtas_event *, int);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rtas_print_event returns and int

void (*cleanup_rtas_event)();
void *(*parse_rtas_event)(char*, int);
void (*rtas_print_event)(FILE *, struct rtas_event *, int);
void (*cleanup_rtas_event)(struct rtas_event *);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanup_rtas_event also returns int

@tyreld tyreld added this to the Release 1.3.14 milestone Feb 25, 2025
@ADITI-MISHRA310
Copy link
Author

I'll resubmit my patch then, thanks.

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 this pull request may close these issues.

3 participants