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

Use portable types to avoid cast warnings #948

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

kmilos
Copy link
Contributor

@kmilos kmilos commented Aug 22, 2023

NB: Requires API version bump (at least when it comes to Windows)

Can of course be solved the other way (explicit casting to unsigned long where needed), if deemed safe.

This is because LLP64 on Windows vs LP64 everywhere else...

@farindk farindk merged commit 4c51939 into strukturag:master Oct 9, 2023
29 of 30 checks passed
@kmilos kmilos deleted the fix_warn_size_t branch October 9, 2023 10:21
@kmilos
Copy link
Contributor Author

kmilos commented Oct 9, 2023

Ooops, looks like I missed one (or it came in in the meantime):

C:/msys64/home/kmilos/libheif/tests/region.cc:393:115: warning: conversion from 'std::vector<unsig
ned char>::size_type' {aka 'long long unsigned int'} to 'long unsigned int' may change value [-Wconv
ersion]
  393 |   err = heif_region_item_add_region_inline_mask_data(region_item1, 20, 50, 64, 3, mask_data.
data(), mask_data.size(), NULL);
      |
        ~~~~~~~~~~~~~~^~

farindk added a commit that referenced this pull request Oct 9, 2023
@farindk
Copy link
Contributor

farindk commented Oct 9, 2023

I have added it.

@kmilos
Copy link
Contributor Author

kmilos commented Oct 16, 2023

@farindk Unfortunately we missed another one (I usually built w/o the JPEG codec):

[79/96] Building CXX object libheif/CMakeFiles/heif.dir/plugins/decoder_jpeg.cc.obj
C:/msys64/home/kmilos/MINGW-packages/mingw-w64-libheif/src/libheif-1.17.0/libheif/plugins/decoder_
jpeg.cc: In function 'heif_error jpeg_decode_image(void*, heif_image**)':
C:/msys64/home/kmilos/MINGW-packages/mingw-w64-libheif/src/libheif-1.17.0/libheif/plugins/decoder_
jpeg.cc:146:64: warning: conversion from 'std::vector<unsigned char>::size_type' {aka 'long long uns
igned int'} to 'long unsigned int' may change value [-Wconversion]
  146 |   jpeg_mem_src(&cinfo, decoder->data.data(), decoder->data.size());
      |                                              ~~~~~~~~~~~~~~~~~~^~

This one actually comes from libjpeg-turbo, so an explicit cast is needed? Haven't checked the API of the IJG libjpeg for this call...

farindk added a commit that referenced this pull request Oct 16, 2023
@farindk
Copy link
Contributor

farindk commented Oct 16, 2023

Yes, seems that we have to cast it.

@kmilos
Copy link
Contributor Author

kmilos commented Oct 17, 2023

@bradh There are also multiple conversion warnings in the uncompressed codec (I haven't been building so far):

/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc: In member function ‘virtual Error Box_uncC::write(StreamWriter&) const’:
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:345:9: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion]
  345 |   flags |= (m_components_little_endian ? 0x80 : 0);
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:346:9: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion]
  346 |   flags |= (m_block_pad_lsb ? 0x40 : 0);
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:347:9: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion]
  347 |   flags |= (m_block_little_endian ? 0x20 : 0);
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:348:9: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion]
  348 |   flags |= (m_block_reversed ? 0x10 : 0);
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:349:9: warning: conversion from ‘int’ to ‘uint8_t’ {aka ‘unsigned char’} may change value [-Wconversion]
  349 |   flags |= (m_pad_unknown ? 0x08 : 0);
      |   ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc: In function ‘Error get_heif_chroma_uncompressed(std::shared_ptr<Box_uncC>&, std::shared_ptr<Box_cmpd>&, heif_chroma*, heif_colorspace*)’:
/home/runner/work/libheif/libheif/libheif/uncompressed_image.cc:474:18: warning: conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion]
  474 |     componentSet |= (1 << component_type);
      |     ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~

@bradh
Copy link
Contributor

bradh commented Oct 17, 2023

We can safely add casts for those. That said, it does seem like a pretty basic false positive if the compiler can't tell that 0x80 or 0 will be unchanged when assigned to a uint8_t.

@kmilos
Copy link
Contributor Author

kmilos commented Oct 17, 2023

That said, it does seem like a pretty basic false positive if the compiler can't tell that 0x80 or 0 will be unchanged when assigned to a uint8_t.

True - that was w/ gcc 9.4 used by mingw CI action on older Ubuntu, and 13.2 here doesn't seem to complain, so it might not be necessary...

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