atomicobject / heatshrink

data compression library for embedded/real-time systems
ISC License
1.31k stars 176 forks source link

Confuse with the code snippet in heatshrink.c #42

Open eeyrw opened 8 years ago

eeyrw commented 8 years ago

static int encoder_sink_read(config cfg, heatshrink_encoder hse, uint8_t data, size_t data_sz) { size_t out_sz = 4096; uint8_t out_buf[out_sz]; memset(out_buf, 0, out_sz); size_t sink_sz = 0; size_t poll_sz = 0; HSE_sink_res sres; HSE_poll_res pres; HSE_finish_res fres; io_handle out = cfg->out;

size_t sunk = 0;
do {
    if (data_sz > 0) {
        sres = heatshrink_encoder_sink(hse, &data[sunk], data_sz - sunk, &sink_sz);
        if (sres < 0) { die("sink"); }
        sunk += sink_sz;
    }

    do {
        pres = heatshrink_encoder_poll(hse, out_buf, out_sz, &poll_sz);
        if (pres < 0) { die("poll"); }
        if (handle_sink(out, poll_sz, out_buf) < 0) die("handle_sink");
    } while (pres == HSER_POLL_MORE);

    if (poll_sz == 0 && data_sz == 0) {
        fres = heatshrink_encoder_finish(hse);
        if (fres < 0) { die("finish"); }
        if (fres == HSER_FINISH_DONE) { return 1; }
    }
} while (sunk < data_sz);
return 0;

}

I found the fact that "data_sz" never have been changed. So branch “if (poll_sz == 0 && data_sz == 0)” won't execute except for zero data_sz. I thinks data_sz here might be substituted with sink_sz due to the symmetry.

silentbicycle commented 8 years ago

You're right that this code is more confusing than it needs to be, particularly since it's the main real example of the library being used. It should be streamlined (as much as possible), and given some comments to explain the subtleties.