Skip to content

Conversation

@grendello
Copy link
Contributor

@grendello grendello commented Sep 1, 2020

When Java.Interop is built with GCC (e.g. on Linux or with MinGW), the
flags Xamarin.Android passes to the compiler cause a handful of warnings
to be shown regarding incomplete field assignment when initializing
structure instances:

java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_type’ [-Wmissing-field-initializers]
  259 |  JavaInteropGCBridge bridge = {0};
      |                                 ^
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_field’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables_count’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables_length’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_domains’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::num_bridge_types’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::mono_java_gc_bridge_info’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_disabled’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_gref_count’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_weak_gref_count’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::Runtime_instance’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::Runtime_gc’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_class’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_init’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_get’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_class’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_add’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_clear’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_log’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_log’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_path’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_path’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_log_level’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_log_level’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_cleanup’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_cleanup’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc: In function ‘_jmethodID* get_add_reference_method(JavaInteropGCBridge*, JNIEnv*, jobject, MonoClass*)’:
java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::bridge_class_kind’ [-Wmissing-field-initializers]
 1283 |  MonoGCBridgeCallbacks   bridge_cbs = {0};
      |                                         ^
java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::is_bridge_object’ [-Wmissing-field-initializers]
java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::cross_references’ [-Wmissing-field-initializers]

Instead of initializing every field, simply use memset to initialize
the structure memory to 0.

Another warning fixed by this commit is one regarding unused function
parameters:

java-interop-gc-bridge-mono.cc:899:93: warning: unused parameter ‘mclass’ [-Wunused-parameter]
  899 | get_add_reference_method (JavaInteropGCBridge *bridge, JNIEnv *env, jobject obj, MonoClass *mclass)
      |                                                                                  ~~~~~~~~~~~^~~~~~

Yet another fixed warning relates to MinGW RELEASE builds only:

java-interop-util.cc: In function ‘char* utf16_to_utf8(const wchar_t*)’:
java-interop/java-interop-util.cc:11:6: warning: unused variable ‘converted_size’ [-Wunused-variable]
   11 |  int converted_size = WideCharToMultiByte (CP_UTF8, 0, widestr, -1, mbstr, required_size, NULL, NULL);
      |      ^~~~~~~~~~~~~~
java-interop/java-interop-util.cc: In function ‘wchar_t* utf8_to_utf16(const char*)’:
java-interop/java-interop-util.cc:23:6: warning: unused variable ‘converted_chars’ [-Wunused-variable]
   23 |  int converted_chars = MultiByteToWideChar (CP_UTF8, 0, mbstr, -1, widestr, required_chars);
      |      ^~~~~~~~~~~~~~~

These warnings are shown because the assert macro used in the above
functions is not compiled in RELEASE build which leaves the variables
mentioned above indeed unused.

grendello added a commit to grendello/xamarin-android that referenced this pull request 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.
grendello added a commit to grendello/xamarin-android that referenced this pull request 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.
int converted_size = WideCharToMultiByte (CP_UTF8, 0, widestr, -1, mbstr, required_size, NULL, NULL);

// Hush a compiler warning about unused variable in RELEASE
(void)converted_size;
Copy link
Member

Choose a reason for hiding this comment

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

how does the compiled code look like? wouldn't it be more readable if we #ifdef'ed the variable declaration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to avoid the #ifdef noise, because you can't just remove the variable conditionally, you'd either have to call WideCharToMultiByte twice in the ifdef blocks, or use an ugly "top-down" ifdef of the form:

#if defined(DEBUG)
int converted_size =
#endif
WideCharToMultiByte (CP_UTF8, 0, widestr, -1, mbstr, required_size, NULL, NULL);

which is an eyesore. Casting the variable to void has no effect on generated code - nothing's there since it's a semantic operation with no side effects. The only downside of leaving the variable (and the cause of the warning) is that you waste some local stack space.

Copy link
Member

@radekdoulik radekdoulik Sep 1, 2020

Choose a reason for hiding this comment

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

It is possible to remove whole variable conditionally as the assert macro will "hide" its use. This compiles just fine:

#include <assert.h>

int main ()
{
#ifndef NDEBUG
  int i;
#endif

  assert(i==0);

  return 0;
}

what if we introduce assert_variable macro like this?

#include <assert.h>

#ifdef NDEBUG
#define assert_variable(x) (__ASSERT_VOID_CAST (0))
#else
#define assert_variable(x) x
#endif

int main ()
{
  assert_variable (int i = 1);

  assert (i == 0);

  return 0;
}

That would avoid the comments and make nice code?

When Java.Interop is built with GCC (e.g. on Linux or with MinGW), the
flags Xamarin.Android passes to the compiler cause a handful of warnings
to be shown regarding incomplete field assignment when initializing
structure instances:

    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_type’ [-Wmissing-field-initializers]
      259 |  JavaInteropGCBridge bridge = {0};
          |                                 ^
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_field’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables_count’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables_length’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_domains’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::BridgeProcessing_vtables’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::num_bridge_types’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::mono_java_gc_bridge_info’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_disabled’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_gref_count’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gc_weak_gref_count’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::Runtime_instance’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::Runtime_gc’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_class’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_init’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::WeakReference_get’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_class’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_add’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::GCUserPeerable_clear’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_log’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_log’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_path’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_path’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_log_level’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_log_level’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::gref_cleanup’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:259:33: warning: missing initializer for member ‘JavaInteropGCBridge::lref_cleanup’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc: In function ‘_jmethodID* get_add_reference_method(JavaInteropGCBridge*, JNIEnv*, jobject, MonoClass*)’:
    java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::bridge_class_kind’ [-Wmissing-field-initializers]
     1283 |  MonoGCBridgeCallbacks   bridge_cbs = {0};
          |                                         ^
    java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::is_bridge_object’ [-Wmissing-field-initializers]
    java-interop-gc-bridge-mono.cc:1283:41: warning: missing initializer for member ‘MonoGCBridgeCallbacks::cross_references’ [-Wmissing-field-initializers]

Instead of initializing every field, simply use `memset` to initialize
the structure memory to `0`.

Another warning fixed by this commit is one regarding unused function
parameters:

    java-interop-gc-bridge-mono.cc:899:93: warning: unused parameter ‘mclass’ [-Wunused-parameter]
      899 | get_add_reference_method (JavaInteropGCBridge *bridge, JNIEnv *env, jobject obj, MonoClass *mclass)
          |                                                                                  ~~~~~~~~~~~^~~~~~

Yet another fixed warning relates to MinGW `RELEASE` builds only:

    java-interop-util.cc: In function ‘char* utf16_to_utf8(const wchar_t*)’:
    java-interop/java-interop-util.cc:11:6: warning: unused variable ‘converted_size’ [-Wunused-variable]
       11 |  int converted_size = WideCharToMultiByte (CP_UTF8, 0, widestr, -1, mbstr, required_size, NULL, NULL);
          |      ^~~~~~~~~~~~~~
    java-interop/java-interop-util.cc: In function ‘wchar_t* utf8_to_utf16(const char*)’:
    java-interop/java-interop-util.cc:23:6: warning: unused variable ‘converted_chars’ [-Wunused-variable]
       23 |  int converted_chars = MultiByteToWideChar (CP_UTF8, 0, mbstr, -1, widestr, required_chars);
          |      ^~~~~~~~~~~~~~~

These warnings are shown because the `assert` macro used in the above
functions is not compiled in `RELEASE` build which leaves the variables
mentioned above indeed unused.
grendello added a commit to grendello/xamarin-android that referenced this pull request 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.
@jonpryor jonpryor merged commit afbc5b3 into dotnet:master Sep 1, 2020
@grendello grendello deleted the warnings-go-away branch September 1, 2020 20:35
grendello added a commit to grendello/xamarin-android that referenced this pull request 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.
@jpobst jpobst added this to the 11.1 (16.9 / 8.9) milestone Sep 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 13, 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.

4 participants