[ad_1]
\n<\/p>\n
I’m not sure what you mean by “correct”; your code is not correct at least in the sense mentione by @SamVarshavchik, and which a compiler will tell you about:<\/p>\n
a.cpp: In function \u2018int getFifoData(unsigned char*)\u2019:\na.cpp:20:29: warning: ISO C++ forbids variable length array \u2018tBuff\u2019 [-Wvla]\n uint8_t tBuff[txBuf.size()];\n<\/code><\/pre>\nIf you want to understand why C++ has no VLA’s read this SO question:<\/p>\n
Why aren’t variable-length arrays part of the C++ standard?<\/p>\n
Issues with the code<\/h1>\n
Here are some issues I believe you should consider.<\/p>\n
Confusing names<\/h3>\n\n- The function
readWrite()<\/code> – does it read? does it write? does it do both? Who knows.<\/li>\n- Don’t clip names. If you want to name a buffer, call it
my_buffer<\/code>, not my_buff<\/code>. Similarly, diagnostic_output<\/code> not diagOutput<\/code>.<\/li>\n- No impromptu initials. What’s a
tBuffer<\/code>? Is it a test buffer? A t<\/strong>ransaction buffer? A t<\/strong>ransmission buffer? A t<\/strong>emporary buffer?<\/li>\n- Not everyone knows what RX means.<\/li>\n
getFifoData()<\/code> – what does it even do? Is its parameter a “FIFO”? That’s not what the parameter name says. And if it is – where is that information we supposedly get? There’s no destination buffer that’s passed for use, nor a container that’s returned.<\/li>\n<\/ul>\nChance of buffer overflow \/ invalid memory access<\/h3>\n\n- Why does
getFifoData()<\/code> take a buffer without also taking its length as well?<\/li>\n- Better yet, why can’t it take a
span<\/code>?<\/li>\n<\/ul>\nUse of dynamically-allocated buffers<\/h3>\n
Both std::vector<\/code> and the variable-length array are dynamically-allocated memory. VLAs have their own issues (see link above), but as for vectors – you would be performing a memory allocation call on each call of this function; and that might be expensive if it gets called a lot.<\/p>\nLogging<\/h3>\n
Printing to the console or to a file is slow. Well, sort of slow, anyway – this is all relative. Now, this happens within an “if” statement, but if you’ve configured your app to log things, you’ll be paying this price on every call to getFifoData()<\/code>.<\/p>\nTime it!<\/h1>\n
Finally, – if you’re worried about performance, time your function, or do it with a profiler. Then you can see how much time it actually takes and whether that’s a problem.<\/p>\n<\/p><\/div>\n