Am Samstag, 17. Dezember 2022, 20:26:42 CET schrieb Gerhard Sittig:
> On Fri, 2022-12-16 at 19:37 +0100, Gerhard Sittig wrote:
> >
> > On Sun, 2022-12-11 at 02:30 +0100, Markus Heidelberg wrote:
> > >
> > > I found some crashes using PulseView with specific command line options.
> > >
> > > https://github.com/sigrokproject/libsigrok/pull/202
> >
> > Am in the process of taking the part that I'm confident about,
> > the trace32 input. Samplerates should be u64 everywhere, no
> > questions asked.
> >
> > Am more reluctant on the bindings, since I don't know and don't
> > use most of them. Would be good to have more eyes on this part of
> > your submission.
>
> Can you check
> https://repo.or.cz/libsigrok/gsi.git/shortlog/refs/heads/binding-u32
> and ACK or NAK the squash that I would apply to take it mainline?
> As usual, alternatively provide something different which I
> should then take instead.
Oh, using "stoui" wasn't a typo, but intention. I should have made this
clearer with a comment at least.
The block in question is actually a replacement for stoui(), which is
missing in the C++ standard:
unsigned long tmp = stoul(value);
if (tmp > std::numeric_limits<uint32_t>::max()) {
throw std::out_of_range("stoui");
}
So I used "stoui" for stoui() that should be used here directly if
existed.
Using "stoul" doesn't fit because the exception occurs outside of the
scope of stoul() and it would be wrong to get an out_of_range("stoul")
exception for a value that fits into unsigned long.
It may seem strange to use "stoui" because there is no real function
named the same, but I thought it to be appropriate.
So my suggestion is to keep "stoui" and add a comment like this at the
beginning of the block:
// Mimic the nonexistent std::stoui().
But I'm open for alternatives.
ACK for rewording of the commit message.
> The other issue that you noticed is that current bindings seem to
> completely lack u32 conversion support. Haven't checked in detail
> which config properties of drivers and inputs are affected. Your
> commit message listed a few, am not aware of seeing other users'
> reports for any of these. But fixing them is worthwile.
I noticed it with the CSV input and checked for occurrences of
g_variant_new_uint32.
Since there is no command line option to pass driver config to
PulseView, I didn't know how these might be affected as well.
Markus
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
