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

camera: Update camera_make_new arguments #5596

Merged
merged 1 commit into from
Nov 18, 2021
Merged

Conversation

kamtom480
Copy link

No description provided.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Was the compiler unable to detect this? Shouldn't this have broken the build with the 1.17 update?

@microdev1
Copy link
Collaborator

Was the compiler unable to detect this? Shouldn't this have broken the build with the 1.17 update?

The warning is being ignored:

../../shared-bindings/camera/Camera.c: In function 'camera_make_new':
../../shared-bindings/camera/Camera.c:68:30: warning: passing argument 2 of 'mp_arg_check_num' makes integer from pointer without a cast [-Wint-conversion]
   68 |     mp_arg_check_num(n_args, kw_args, 0, 0, false);
      |                              ^~~~~~~
      |                              |
      |                              mp_map_t * {aka struct _mp_map_t *}
In file included from ../../shared-bindings/camera/Camera.c:28:
../../py/runtime.h:87:59: note: expected 'size_t' {aka 'unsigned int'} but argument is of type 'mp_map_t *' {aka 'struct _mp_map_t *'}
   87 | static inline void mp_arg_check_num(size_t n_args, size_t n_kw, size_t n_args_min, size_t n_args_max, bool takes_kw) {
      |                                                    ~~~~~~~^~~~
../../shared-bindings/camera/Camera.c: At top level:
../../shared-bindings/camera/Camera.c:130:17: warning: initialization of 'void * (*)(const mp_obj_type_t *, size_t,  size_t,  void * const*)' {aka 'void * (*)(const struct _mp_obj_type_t *, unsigned int,  unsigned int,  void * const*)'} from incompatible pointer type 'void * (*)(const mp_obj_type_t *, size_t,  void * const*, mp_map_t *)' {aka 'void * (*)(const struct _mp_obj_type_t *, unsigned int,  void * const*, struct _mp_map_t *)'} [-Wincompatible-pointer-types]
  130 |     .make_new = camera_make_new,
      |                 ^~~~~~~~~~~~~~~
../../shared-bindings/camera/Camera.c:130:17: note: (near initialization for 'camera_type.make_new')

Copy link
Collaborator

@microdev1 microdev1 left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks @kamtom480.

@jepler jepler merged commit 932131b into adafruit:main Nov 18, 2021
@jepler
Copy link
Member

jepler commented Nov 18, 2021

Thank you! I'm sorry I didn't catch this during the 1.17 merge, but as you may have guessed I was relying solely on fatal compiler diagnostics to find all the relevant sites.

If possible, as a fresh PR, can you enable -Werror & other diagnostic flags so that we catch these errors during builds? If there are sites in the SDK that have diagnostics, they can be selectively disabled; there are examples of this in the atmel-samd port.

If you're able to take on that additional work, thank you again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants