Skip to content

Conversation

@grendello
Copy link
Contributor

@grendello grendello commented Sep 1, 2020

Context: dotnet/java-interop#703

This commit fixes a handful of warnings reported by GCC and clang when
building for different platforms.

monodroid-glue.cc:1609:8: warning: declaration shadows a static data member of 'xamarin::android::internal::MonodroidRuntime' [-Wshadow]
void *api_dso_handle = nullptr;
              ^
monodroid-glue.cc:102:25: note: previous declaration is here
void *MonodroidRuntime::api_dso_handle = nullptr;

Fix by renaming the local variable. No functionality changes result
from this fix.

MinGW builds report:

xa-internal-api.cc: In member function ‘virtual mono_bool xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state(const char*, mono_bool*)’:
xa-internal-api.cc:24:86: warning: unused parameter ‘ifname’ [-Wunused-parameter]
   24 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state (const char *ifname, mono_bool *is_up)
      |                                                                          ~~~~~~~~~~~~^~~~~~
xa-internal-api.cc:24:105: warning: unused parameter ‘is_up’ [-Wunused-parameter]
   24 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state (const char *ifname, mono_bool *is_up)
      |                                                                                              ~~~~~~~~~~~^~~~~
xa-internal-api.cc: In member function ‘virtual mono_bool xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast(const char*, mono_bool*)’:
xa-internal-api.cc:34:96: warning: unused parameter ‘ifname’ [-Wunused-parameter]
   34 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast (const char *ifname, mono_bool *supports_multicast)
      |                                                                                    ~~~~~~~~~~~~^~~~~~
xa-internal-api.cc:34:115: warning: unused parameter ‘supports_multicast’ [-Wunused-parameter]
   34 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast (const char *ifname, mono_bool *supports_multicast)
      |                                                                                                        ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
xa-internal-api.cc: In member function ‘virtual int xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_dns_servers(void**)’:
xa-internal-api.cc:44:66: warning: unused parameter ‘dns_servers_array’ [-Wunused-parameter]
   44 | MonoAndroidInternalCalls_Impl::monodroid_get_dns_servers (void **dns_servers_array)
      |                                                           ~~~~~~~^~~~~~~~~~~~~~~~~
xa-internal-api.cc: In member function ‘virtual int xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_getifaddrs(_monodroid_ifaddrs**)’:
xa-internal-api.cc:54:82: warning: unused parameter ‘ifap’ [-Wunused-parameter]
   54 | MonoAndroidInternalCalls_Impl::monodroid_getifaddrs (struct _monodroid_ifaddrs **ifap)
      |                                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
xa-internal-api.cc: In member function ‘virtual void xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_freeifaddrs(_monodroid_ifaddrs*)’:
xa-internal-api.cc:64:82: warning: unused parameter ‘ifa’ [-Wunused-parameter]
   64 | MonoAndroidInternalCalls_Impl::monodroid_freeifaddrs (struct _monodroid_ifaddrs *ifa)
      |                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

The fix is to ignore the unused parameters only during Windows builds
by using the C++17 attribute [[maybe_unused]]

Another Windows-only warning:

embedded-assemblies.cc: In static member function ‘static ssize_t xamarin::android::internal::EmbeddedAssemblies::do_read(int, void*, size_t)’:
embedded-assemblies.cc:663:26: warning: conversion from ‘size_t’ {aka ‘long long unsigned int’} to ‘unsigned int’ may change value [-Wconversion]
  663 |   ret = ::read (fd, buf, count);
      |

It is caused by read(2) declared in MinGW headers with the third
parameter using unsigned int type instead of the standard size_t.
Fixed by casting properly on Windows builds.

Context: dotnet/java-interop#703

This commit fixes a handful of warnings reported by GCC and clang when
building for different platforms.

    monodroid-glue.cc:1609:8: warning: declaration shadows a static data member of 'xamarin::android::internal::MonodroidRuntime' [-Wshadow]
    void *api_dso_handle = nullptr;
                  ^
    monodroid-glue.cc:102:25: note: previous declaration is here
    void *MonodroidRuntime::api_dso_handle = nullptr;

Fix by renaming the local variable.  No functionality changes result
from this fix.

MinGW builds report:

    xa-internal-api.cc: In member function ‘virtual mono_bool xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state(const char*, mono_bool*)’:
    xa-internal-api.cc:24:86: warning: unused parameter ‘ifname’ [-Wunused-parameter]
       24 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state (const char *ifname, mono_bool *is_up)
          |                                                                          ~~~~~~~~~~~~^~~~~~
    xa-internal-api.cc:24:105: warning: unused parameter ‘is_up’ [-Wunused-parameter]
       24 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_up_state (const char *ifname, mono_bool *is_up)
          |                                                                                              ~~~~~~~~~~~^~~~~
    xa-internal-api.cc: In member function ‘virtual mono_bool xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast(const char*, mono_bool*)’:
    xa-internal-api.cc:34:96: warning: unused parameter ‘ifname’ [-Wunused-parameter]
       34 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast (const char *ifname, mono_bool *supports_multicast)
          |                                                                                    ~~~~~~~~~~~~^~~~~~
    xa-internal-api.cc:34:115: warning: unused parameter ‘supports_multicast’ [-Wunused-parameter]
       34 | MonoAndroidInternalCalls_Impl::monodroid_get_network_interface_supports_multicast (const char *ifname, mono_bool *supports_multicast)
          |                                                                                                        ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~
    xa-internal-api.cc: In member function ‘virtual int xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_get_dns_servers(void**)’:
    xa-internal-api.cc:44:66: warning: unused parameter ‘dns_servers_array’ [-Wunused-parameter]
       44 | MonoAndroidInternalCalls_Impl::monodroid_get_dns_servers (void **dns_servers_array)
          |                                                           ~~~~~~~^~~~~~~~~~~~~~~~~
    xa-internal-api.cc: In member function ‘virtual int xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_getifaddrs(_monodroid_ifaddrs**)’:
    xa-internal-api.cc:54:82: warning: unused parameter ‘ifap’ [-Wunused-parameter]
       54 | MonoAndroidInternalCalls_Impl::monodroid_getifaddrs (struct _monodroid_ifaddrs **ifap)
          |                                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
    xa-internal-api.cc: In member function ‘virtual void xamarin::android::internal::MonoAndroidInternalCalls_Impl::monodroid_freeifaddrs(_monodroid_ifaddrs*)’:
    xa-internal-api.cc:64:82: warning: unused parameter ‘ifa’ [-Wunused-parameter]
       64 | MonoAndroidInternalCalls_Impl::monodroid_freeifaddrs (struct _monodroid_ifaddrs *ifa)
          |                                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~

The fix is to ignore the unused parameters only during Windows builds
by using the C++17 attribute `[[maybe_unused]]`

Another Windows-only warning:

    embedded-assemblies.cc: In static member function ‘static ssize_t xamarin::android::internal::EmbeddedAssemblies::do_read(int, void*, size_t)’:
    embedded-assemblies.cc:663:26: warning: conversion from ‘size_t’ {aka ‘long long unsigned int’} to ‘unsigned int’ may change value [-Wconversion]
      663 |   ret = ::read (fd, buf, count);
          |

It is caused by `read(2)` declared in MinGW headers with the third
parameter using `unsigned int` type instead of the standard `size_t`.
Fixed by casting properly on Windows builds.
<PackageReference Include="System.Runtime" Version="4.3.1" />
<PackageReference Include="System.Runtime.InteropServices" Version="4.3.0" />
<PackageReference Include="XliffTasks" Version="1.0.0-beta.19252.1" PrivateAssets="all" />
<PackageReference Include="XliffTasks" Version="1.0.0-beta.20206.1" PrivateAssets="all" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this was updated in the context of C++ warning removal…

Copy link
Contributor Author

@grendello grendello Sep 2, 2020

Choose a reason for hiding this comment

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

Because the PR updates the Java.Interop submodule, whose master branch uses this version of XliffTasks and the difference in versions broke the build of XA with:

 Installing System.Reflection.Emit.ILGeneration 4.3.0.
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Detected package downgrade: XliffTasks from 1.0.0-beta.20206.1 to 1.0.0-beta.19252.1. Reference the package directly from the project to select a different version.  [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error :  Xamarin.Android.Build.Tasks -> Java.Interop.Tools.Cecil -> Java.Interop.Localization -> XliffTasks (>= 1.0.0-beta.20206.1)  [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error :  Xamarin.Android.Build.Tasks -> XliffTasks (>= 1.0.0-beta.19252.1) [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error : Detected package downgrade: XliffTasks from 1.0.0-beta.20206.1 to 1.0.0-beta.19252.1. Reference the package directly from the project to select a different version.  [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error :  Xamarin.Android.Build.Tasks -> Java.Interop.Tools.JavaCallableWrappers -> Java.Interop.Localization -> XliffTasks (>= 1.0.0-beta.20206.1)  [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]
/home/grendel/devel/mono/mono-master/lib/mono/msbuild/Current/bin/NuGet.targets(128,5): error :  Xamarin.Android.Build.Tasks -> XliffTasks (>= 1.0.0-beta.19252.1) [/home/grendel/vc/xamarin/xamarin-android-worktrees/warnings-go-away/Xamarin.Android.sln]

Also, https://discordapp.com/channels/732297728826277939/732297837953679412/750350008464965632

@grendello
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jonpryor jonpryor merged commit e1f2558 into dotnet:master Sep 2, 2020
@grendello grendello deleted the warnings-go-away branch September 2, 2020 16:53
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants