[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Discuss-gnuradio] Strange FEC Mod2 Code
From: |
Michael Dickens |
Subject: |
[Discuss-gnuradio] Strange FEC Mod2 Code |
Date: |
Thu, 29 Jun 2017 13:37:55 -0400 |
For kicks and grins, I sometimes build GR using "-Wall" and see what
sort of fun warnings come out when using various versions of Clang and
GCC. In general, GR (and UHD & Volk) build pretty cleanly.
The latest such compiling turned up this interesting piece of code, now
in gr-fec/lib/fec_mtrx_impl.cc:448-450
<
https://github.com/gnuradio/gnuradio/blob/master/gr-fec/lib/fec_mtrx_impl.cc#L448
>
{{{
// take care of mod 2
int value_cast_as_int = static_cast<int>(value);
int temp_value = abs(fmod(value_cast_as_int,2));
}}}
Here's the original commit: <
https://github.com/gnuradio/gnuradio/commit/61ddcf22cd3eb7bf1024926c74093f2657872bf8#diff-3f87944036e301314b1f7452687e40d9R1489
>. I'm cc'ing the folks who put in this commit.
As I read it, all this code does is a glorified "mod2", setting
"temp_value" to the integer remainder when dividing "value_cast_as_int"
by 2. Why use the fancy "fmod" and "abs", which require type
conversation from int to double then back to int?
Since "value_cast_as_int" is an int, and "2" is an int, why not just do
either
{{{
int temp_value = value_cast_as_int % 2;
}}}
or
{{{
int temp_value = value_cast_as_int & 1;
}}}
These seem both more clear and faster due to no type conversion. One
could get fancy using std::div, but since all we seem to care about is
the remainder, returning both X / Y and X % Y seems unnecessary.
Am I missing something? Thanks for your thoughts. - MLD
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Discuss-gnuradio] Strange FEC Mod2 Code,
Michael Dickens <=