Skip to content

[pull] master from libretro:master#947

Merged
pull[bot] merged 3 commits intoAlexandre1er:masterfrom
libretro:master
Apr 21, 2026
Merged

[pull] master from libretro:master#947
pull[bot] merged 3 commits intoAlexandre1er:masterfrom
libretro:master

Conversation

@pull
Copy link
Copy Markdown

@pull pull Bot commented Apr 21, 2026

See Commits and Changes for more details.


Created by pull[bot] (v2.0.0-alpha.4)

Can you help keep this open source service alive? 💖 Please sponsor : )

GCC on 10.5 PPC warned:

  iohidmanager_hid.c:691: warning: comparison between signed and
  unsigned

pad_connection_pad_init() at input/connect/joypad_connection.c:397
returns int32_t and can return -1 on allocation failure.  Four HID
drivers store that return value in an adapter struct field and use
it for array indexing.  Three got the type wrong:

  iohidmanager_hid.c : uint32_t slot (warning reported here)
  btstack_hid.c      : uint32_t slot (same structural mismatch,
                       silent - no == -1 check at call site)
  libusb_hid.c       : int slot      (compiles clean, but width is
                       implicit rather than explicit)
  wiiusb_hid.c       : int32_t slot  (correct)

Align all four on int32_t, matching the return type of
pad_connection_pad_init and wiiusb_hid.c's precedent.

Array-indexing call sites (hid->slots[adapter->slot], hid->hats
[adapter->slot][...], etc.) are unaffected - signed and unsigned
integer types index arrays identically through integer promotion
to ptrdiff_t.  The only behavioural effect is that slot == -1
checks (iohidmanager_hid.c:691, libusb_hid.c:313) are now signed/
signed comparisons and actually do what they look like they do.

Previously the iohidmanager check "worked" only because the
promotion rules happen to coerce (int)-1 to UINT32_MAX for the
comparison - accidental correctness.
21 preprocessor blocks in ui_cocoa.m guarded [... release] and
[... autorelease] calls behind `#ifndef HAVE_COCOA_METAL`.  Plus
one -dealloc override gated on the same.  This was using
HAVE_COCOA_METAL as a proxy for "are we compiled under ARC?".

The proxy is brittle:

  Build target                 ARC?  MRR?
  qb/make (any)                no    YES (ui_cocoa.m always MRR;
                                      only metal.m + mfi_joypad.m
                                      get -fobjc-arc per Makefile
                                      lines 275-276)
  RetroArch.xcodeproj          YES   no  (BaseConfig.xcconfig:182
  RetroArch_Metal.xcodeproj    YES   no   CLANG_ENABLE_OBJC_ARC=YES
  RetroArch_OSX107.xcodeproj   YES   no   cascades down)
  RetroArch_PPC.xcodeproj      no    YES (explicit override to NO)

The HAVE_COCOA_METAL-based guard happens to be right for 3 of
these 5 by coincidence (Metal xcodeproj + PPC xcodeproj + qb/make),
but wrong for the two that flip the correlation.  The clang-
documented discriminator is __has_feature(objc_arc), which is
exactly true under ARC and false under MRR regardless of other
build options.

Add three macros in cocoa_defines.h that expand to the right thing
either way:

  RARCH_RELEASE(x)       [(x) release]    /  ((void)0) under ARC
  RARCH_AUTORELEASE(x)   [(x) autorelease] / ((void)0) under ARC
  RARCH_SUPER_DEALLOC()  [super dealloc]   / ((void)0) under ARC

Wrapped in the standard __has_feature polyfill so GCC 4.0 (Xcode
3.1, which predates ARC and __has_feature both) falls through to
the MRR branch, which is correct.

Replace 21 conditional release/autorelease blocks with unconditional
macro calls.  Gate the -dealloc override on #if !__has_feature
(objc_arc) instead of #ifndef HAVE_COCOA_METAL - ARC auto-generates
-dealloc from strong ivars and forbids explicit overrides that just
call [super dealloc], so the method stays conditional but on the
correct axis.

The one remaining #ifndef HAVE_COCOA_METAL site (now line 646,
the addSubview:[CocoaView get] / makeFirstResponder: block) is a
legitimate platform branch - Metal installs CocoaView via
apple_platform.renderView through a different codepath - and
stays put.

Net change: -10 LoC, -21 preprocessor blocks, zero runtime
behaviour change on any of the five build configurations.
-[CocoaView init] at cocoa_common.m:254 had an #if defined(HAVE_COCOA)
block that declared a stack-local ui_window_cocoa_t, assigned its
.data field to self, then let the variable go out of scope without
passing, storing, or otherwise referencing it.  Pure dead code.

Confirmed unused: ui_window_cocoa_t is the payload type for the
ui_window_cocoa_* driver vtable in ui_cocoa.m, but that vtable's
init function returns NULL, so the other entry points never see
a valid ui_window_cocoa_t anyway - unrelated pre-existing plumbing
issue with the ui_window driver system, not something this commit
addresses.

Removing the block also lets the two surrounding #if defined(OSX)
blocks merge into one, eliminating one preprocessor conditional.
Zero runtime change; the generated code for -[CocoaView init] is
identical modulo the unused stack slot.

Caught during a HAVE_COCOA_METAL vs HAVE_COCOA convergence sweep
(following 492b9c4) - one of the few remaining HAVE_COCOA-only
branches turned out to be dead code rather than a real platform
divergence.
@pull pull Bot locked and limited conversation to collaborators Apr 21, 2026
@pull pull Bot added the ⤵️ pull label Apr 21, 2026
@pull pull Bot merged commit b35a4c2 into Alexandre1er:master Apr 21, 2026
31 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant