Skip to content

Make bt_field_blob_get_length return size_t instead of uint64_t #120

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kraj
Copy link

@kraj kraj commented Mar 21, 2025

Fixes errors e.g.
| ../../git/src/cpp-common/bt2/field.hpp:1139:82: error: non-constant-expression cannot be narrowed from type 'std::uint64_t' (aka 'unsigned long long') to 'size_type' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]
| 1139 | return {internal::CommonBlobFieldSpec::data(this->libObjPtr()), this->length()};
| | ^~~~~~~~~~~~~~
| ../../git/src/plugins/ctf/common/src/msg-iter.cpp:744:56: note: in instantiation of member function 'bt2::CommonBlobField<bt_field>::data' requested here
| 744 | std::memcpy(&this->_stackTopCurSubField().asBlob().data()[_mCurBlobFieldDataOffset],
| | ^
| ../../git/src/cpp-common/bt2/field.hpp:1139:82: note: insert an explicit cast to silence this issue
| 1139 | return {internal::CommonBlobFieldSpec::data(this->libObjPtr()), this->length()};
| | ^~~~~~~~~~~~~~
| | static_cast<size_type>( )

@eepp
Copy link
Member

eepp commented Mar 24, 2025

Thank you.

If you don't mind, could you create an issue on our official bug tracker instead? If you can't, I will.

May I also get your configure command line?

This specific fix isn't the right one. bt2::CommonBlobField::length() has every right to return std::uint64_t: it wraps a uint64_t-returning function. The constructor of bt2s::span expects std::size_t; that's where you want to cast this->length().

@kraj
Copy link
Author

kraj commented Mar 24, 2025

@eepp this issue happens when building for riscv32 targets using clang-20 compiler. I think it will also fail on other 32bit platforms too.

@kraj
Copy link
Author

kraj commented Mar 24, 2025

I do not have account on the bug tracker and creating new is sent to overlords for approval :)

…t2s::span

Fixes errors e.g.
| ../../git/src/cpp-common/bt2/field.hpp:1139:82: error: non-constant-expression cannot be narrowed from type 'std::uint64_t' (aka 'unsigned long long') to 'size_type' (aka 'unsigned int') in initializer list [-Wc++11-narrowing]
|  1139 |         return {internal::CommonBlobFieldSpec<LibObjT>::data(this->libObjPtr()), this->length()};
|       |                                                                                  ^~~~~~~~~~~~~~
| ../../git/src/plugins/ctf/common/src/msg-iter.cpp:744:56: note: in instantiation of member function 'bt2::CommonBlobField<bt_field>::data' requested here
|   744 |     std::memcpy(&this->_stackTopCurSubField().asBlob().data()[_mCurBlobFieldDataOffset],
|       |                                                        ^
| ../../git/src/cpp-common/bt2/field.hpp:1139:82: note: insert an explicit cast to silence this issue
|  1139 |         return {internal::CommonBlobFieldSpec<LibObjT>::data(this->libObjPtr()), this->length()};
|       |                                                                                  ^~~~~~~~~~~~~~
|       |                                                                                  static_cast<size_type>( )

Signed-off-by: Khem Raj <[email protected]>
@eepp
Copy link
Member

eepp commented Mar 25, 2025

In fact don't worry about the bug tracker, this is so simple we'll fix it now.

The patch is fine. The "right" cast is probably something like

typename bt2s::span<typename internal::CommonBlobFieldSpec<LibObjT>::Data>::size_type

(lol) but whatever.

However can you move this to a Gerrit change? You already have an account there.

@kraj
Copy link
Author

kraj commented Mar 25, 2025

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.

2 participants