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 -Wformat violations #81618

Merged
merged 1 commit into from
Feb 4, 2023

Conversation

am11
Copy link
Member

@am11 am11 commented Feb 3, 2023

Removes -Wno-format by defining formats in minipal/types.h and using it where it's needed to fix violations.

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Feb 3, 2023
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
Copy link
Member

Choose a reason for hiding this comment

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

@LakshanF This is the work I was alluding to regarding printf.

Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT left a comment

Choose a reason for hiding this comment

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

:shipit:

Thank you!

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@AaronRobinsonMSFT
Copy link
Member

Failure appears to be known.

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit 052bd58 into dotnet:main Feb 4, 2023
#ifndef HAVE_MINIPAL_TYPES_H
#define HAVE_MINIPAL_TYPES_H

#if defined(TARGET_32BIT) || defined(TARGET_OSX) || defined(TARGET_WINDOWS)
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @lambdageek we can move these

set(EGLIB_GSIZE_FORMAT "\"Iu\"")
else()
set(EGLIB_GSIZE_FORMAT "\"u\"")
endif()
else()
set(EGLIB_BREAKPOINT "G_STMT_START { raise(SIGTRAP); } G_STMT_END")
if(GCC)
set(EGLIB_GNUC_UNUSED "__attribute__((__unused__))")
set(EGLIB_GNUC_NORETURN "__attribute__((__noreturn__))")
if(HOST_AMD64 OR HOST_X86)
set(EGLIB_BREAKPOINT "G_STMT_START { __asm__(\"int \$03\"); } G_STMT_END")
endif()
endif()
set(EGLIB_PATHSEP "/")
set(EGLIB_SEARCHSEP ":")
set(EGLIB_OS "UNIX")
set(EGLIB_PIDTYPE "int")
set(EGLIB_GSIZE_FORMAT "\"zu\"")
definitions here with a similar workaround for HOST_ vs. TARGET_
# TODO: remove the mono-style HOST_ variable checks once Mono is using eng/native/configureplatform.cmake to define the CLR_CMAKE_TARGET_ defines

(I just realized that the macros in this file should be prefixed with MINIPAL_ for consistency)

@am11 am11 deleted the feature/native/warning-fixes branch February 4, 2023 12:42
@ghost ghost locked as resolved and limited conversation to collaborators Mar 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-PAL-coreclr community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants