>> Also, I don't think >> the new logic for the ctrl/c variables is an improvement: >> >> 1. iter->ctrlc is intialized with '8' (even in the uncompressed case, >> which is confusing). Any time you initialize with something not 0 or >> 1, it's a magic number, and here it's far from where the loop variable >> is used. This is harder to read. > > `iter->ctrlc` is used to record the value of `ctrl` in pglz_decompress at the end of > the last iteration(or loop). In the pglz_decompress, `ctrlc`’s valid value is 0~7, > When `ctrlc` reaches 8, a control byte is read from the source > buffer to `ctrl` then set `ctrlc` to 0. And a control bytes should be read from the > source buffer to `ctrlc` on the first iteration. So `iter->ctrlc` should be intialized with '8'.
My point here is it looks strange out of context, but "0" looked normal. Maybe a comment in init_detoast_buffer(), something like "8 means read a control byte from the source buffer on the first iteration, see pg_lzdecompress_iterate()".
Or, possibly, we could have a macro like INVALID_CTRLC. That might even improve the readability of the original function. This is just an idea, and maybe others would disagree, so you don't need to change it for now.
All in all, the idea is much better than a magic number 8. So, I've implemented it.
At this point, there are no functional things that I think we need to change. It's close to ready-for-committer. For the next version, I'd like you go through the comments and edit for grammar, spelling, and clarity as you see fit. I know you're not a native speaker of English, so I can help you with anything that remains.
I've tried my best to improve the comments, but there should be room for further improvement
I hope you can help me perfect it.
Also note we use braces on their own lines { like this }
