discuss-gnuradio
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]