Skip to content

Commit f93853d

Browse files
author
Dan Albert
committed
Fix C incompatibilities.
Bug: android/ndk#1920 Test: clang --sysroot out/soong/ndk/sysroot out/soong/ndk/sysroot/usr/include/**/*.h Change-Id: Ic1647135769fd8b4699b6fa89b26ebee51bed18d
1 parent 9d04fe2 commit f93853d

1 file changed

Lines changed: 36 additions & 4 deletions

File tree

include/android/surface_control.h

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,28 @@ void ASurfaceTransaction_setColor(ASurfaceTransaction* _Nonnull transaction,
369369
float b, float alpha, enum ADataSpace dataspace)
370370
__INTRODUCED_IN(29);
371371

372+
// These APIs (setGeometry and setCrop) were originally written in a
373+
// C-incompatible form using references instead of pointers, and the OS shipped
374+
// that version for years before it was noticed. Fortunately the compiled code
375+
// for callers is the same regardless of whether it's a pointer or a reference,
376+
// so we can declare this as a nonnull pointer for C and keep the existing C++
377+
// decl and definition.
378+
//
379+
// We could alternatively change the decl and the definition to both be a
380+
// pointer (with an inline definition using references to preserve source compat
381+
// for existing C++ callers), but that requires changing the definition of an
382+
// API that has been in the OS for years. It's theoretically a safe change, but
383+
// without being able to prove it that's a very big risk to take. By keeping the
384+
// C-compatibility hack in the header, we can be sure that we haven't changed
385+
// anything for existing callers. By definition there were no C users of the
386+
// reference-based decl; if there were any C callers of the API at all, they were
387+
// using the same workaround that is now used below.
388+
//
389+
// Even if this workaround turns out to not work for C, there's no permanent
390+
// damage done to the platform (unlike if we were to change the definition). At
391+
// worst it continues to work for C++ (since the preprocessed header as seen by
392+
// C++ hasn't changed, nor has the definition) and continues to not work for C.
393+
372394
/**
373395
* \param source The sub-rect within the buffer's content to be rendered inside the surface's area
374396
* The surface's source rect is clipped by the bounds of its current buffer. The source rect's width
@@ -390,9 +412,14 @@ void ASurfaceTransaction_setColor(ASurfaceTransaction* _Nonnull transaction,
390412
* properties at once.
391413
*/
392414
void ASurfaceTransaction_setGeometry(ASurfaceTransaction* _Nonnull transaction,
393-
ASurfaceControl* _Nonnull surface_control, const ARect& source,
394-
const ARect& destination, int32_t transform)
395-
__INTRODUCED_IN(29);
415+
ASurfaceControl* _Nonnull surface_control,
416+
#if defined(__cplusplus)
417+
const ARect& source, const ARect& destination,
418+
#else
419+
const ARect* _Nonnull source,
420+
const ARect* _Nonnull destination,
421+
#endif
422+
int32_t transform) __INTRODUCED_IN(29);
396423

397424
/**
398425
* Bounds the surface and its children to the bounds specified. The crop and buffer size will be
@@ -404,7 +431,12 @@ void ASurfaceTransaction_setGeometry(ASurfaceTransaction* _Nonnull transaction,
404431
* Available since API level 31.
405432
*/
406433
void ASurfaceTransaction_setCrop(ASurfaceTransaction* _Nonnull transaction,
407-
ASurfaceControl* _Nonnull surface_control, const ARect& crop)
434+
ASurfaceControl* _Nonnull surface_control,
435+
#if defined(__cplusplus)
436+
const ARect& crop)
437+
#else
438+
const ARect* _Nonnull crop)
439+
#endif
408440
__INTRODUCED_IN(31);
409441

410442
/**

0 commit comments

Comments
 (0)