On Mon, Apr 9, 2012 at 2:02 PM, Nick Foster <n...@ettus.com> wrote: > On Mon, Apr 9, 2012 at 10:48 AM, Marcus D. Leech <mle...@ripnet.com> wrote: >> >> On 04/09/2012 01:38 PM, Tom Rondeau wrote: >>> >>> On Sat, Apr 7, 2012 at 10:12 PM, Marcus D. Leech<mle...@ripnet.com> >>> wrote: >>>> >>>> Just looking at this function: >>>> >>>> correlate_access_code_bb >>>> >>>> In the method set_access_code, it takes a string. Which should be ASCII >>>> '1' >>>> and '0' characters to represent the binary sequence being >>>> correlated against. >>>> >>>> Here's a little beauty of a code snippet: >>>> >>>> d_access_code = 0; >>>> for (unsigned i=0; i< 64; i++){ >>>> d_access_code<<= 1; >>>> if (i< len) >>>> d_access_code |= access_code[i]& 1; // look at LSB only >>>> } >>>> >>>> This relies on the fact that ASCII '1' and '0' happen to have low-order >>>> bits >>>> of the right "flavour". This is insanely dirty and gross and I can't >>>> believe we let this nonsense in the code base. >>>> >>>> There's no reason not to do the right thing here. >>>> >>>> >>>> -- >>>> Marcus Leech >>>> Principal Investigator >>>> Shirleys Bay Radio Astronomy Consortium >>>> http://www.sbrac.org >>> >>> >>> Want to submit a patch? >>> >>> Tom >>> >>> >> Attached. > > > While you're patching correlate_access_code_bb, please patch > correlate_access_code_tag_bb with the attached patch. > > --n
So my guess is that the use of the binary & operator is to avoid the need for an if/if else/else branching check. It was most likely done for efficiency. So while this patch might be the "right" way to do it from a code perspective, it could result in slower code (on certain machines that don't handle branch prediction well). It does make assumptions about the correctness of the access code, though. Tom _______________________________________________ Discuss-gnuradio mailing list Discuss-gnuradio@gnu.org https://lists.gnu.org/mailman/listinfo/discuss-gnuradio