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 some XS9 warnings #6066

Merged
merged 2 commits into from
Nov 6, 2024

Conversation

rosslagerwall
Copy link
Contributor

Fix a warning reported by newer systemd as well as a build warning I noticed when building with a newer GCC.

GCC correctly reports "pointer targets differ in signedness".

Signed-off-by: Ross Lagerwall <[email protected]>
Newer systemd warns that the "syslog" StandardOutput/StandardError type
is deprecated and automatically uses the journal instead. Fix this by
removing the explicit setting of StandardOutput/StandardError. Instead,
the service will use the default values configured in systemd
(DefaultStandardOutput/DefaultStandardError) which for a XenServer
system will result in the output going to rsyslog.

Signed-off-by: Ross Lagerwall <[email protected]>
@rosslagerwall
Copy link
Contributor Author

There doesn't seem to be that much consistency between the settings of StandardOutput / StandardError in the various toolstack service files. I'm not sure whether this is intentional or not.

@rosslagerwall
Copy link
Contributor Author

message-switch.service is also affected by this issue but its service file is not in this repo for some (unknown to me) reason.

@psafont
Copy link
Member

psafont commented Oct 18, 2024

The service files are in xapi or not, depending on whether they had the service file in the repo or the spec file, back when they were split from xapi. I don't think it's important, and I think they should be hosted in this repository.

@@ -31,7 +31,7 @@ value is_all_zeros(value string, value length)
for (i = len / 4; i > 0; i--)
if (*p++ != 0)
goto notallzero;
s = (unsigned char *) p;
s = (const char *) p;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not a regression but looking at the above code even p should be a const pointer. There's also the assumption that sizeof(int) == 4 but I suppose code is not intended to be that portable, although a uint32_t pointer would maybe be better... or replace the 4 with sizeof(int) or sizeof(*p).

Copy link
Contributor

@Vincent-lau Vincent-lau left a comment

Choose a reason for hiding this comment

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

does this needs to be changed as well?

(** [start_transient ?env ?properties ~service cmd args] generates and
starts a transient systemd [service] that will execute [cmd args].
stdout/stderr from the service is redirected to syslog with
[service] as syslog key. Additional [properties] can be specified
that are written into the systemd unit file's [Service] section. By
default the service is not auto-restarted when it fails, and there
is a 10s timeout between SIGTERM and SIGKILL on stop. On failure it
raises [Spawn_internal_error(stderr, stdout, Unix.process_status)]

@robhoes robhoes added this pull request to the merge queue Nov 5, 2024
github-merge-queue bot pushed a commit that referenced this pull request Nov 5, 2024
Fix a warning reported by newer systemd as well as a build warning I
noticed when building with a newer GCC.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2024
@psafont psafont added this pull request to the merge queue Nov 6, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 6, 2024
@psafont psafont added this pull request to the merge queue Nov 6, 2024
Merged via the queue into xapi-project:master with commit 3d3bd23 Nov 6, 2024
15 checks passed
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.

5 participants