sahlberg / libnfs

NFS client library
Other
510 stars 200 forks source link

Fix rpc_nfs3_read_task: Failing to return data #472

Closed amulyan13 closed 2 months ago

amulyan13 commented 2 months ago

Issue: rpc_nfs3_read_task API does not populate the data fetched from the server to the buffer passed by the caller. It always copies '0' bytes irrespective of the size requested by the user or the size returned by the server. rpc_nfs3_read_task(get_rpc_ctx(), readfile_callback, testbuf, 64, &args, this) <<< testbuf is always null even on successful 0+ read.

RCA: This bug was introduced as part of commit https://github.com/sahlberg/libnfs/commit/bf010958226678cf07854a365ccefb872e499775 where the long reads were cut to the size requested by the caller.

[lib/socket.c]: if (rpc->pdu->read_count > rpc->pdu->requested_read_count) { rpc->pdu->read_count = rpc->pdu->requested_read_count; }

The requested_read_count has the default value of 0 and it is not set anywhere to the size requested by the user. This is causing only 0 bytes to be copied to the input buffer and hence the caller can not see any data read from the file.

                                    if (rpc->pdu->read_count > rpc->pdu->requested_read_count) {
                                            rpc->pdu->read_count = rpc->pdu->requested_read_count;  <<< This is now set to 0
                                    }
                                    if (count > rpc->pdu->read_count) {
                                            count = rpc->pdu->read_count;
                                    }
                                    if (rpc->pdu->in.len > rpc->pdu->read_count) {
                                            /* we got a short read */
                                            rpc->pdu->in.len = rpc->pdu->read_count;
                                    }
                                    tmp_count = count;
                                    if (rpc->pdu->in.len <= count) {
                                            tmp_count = count;
                                    }
                                    memcpy(rpc->pdu->in.buf, &rpc->inbuf[pos], tmp_count); << We end up copying only 0 bytes.

Fix: I have set the rpc->pdu->requested_read_count to the user requested value before queuing the pdu.

sahlberg commented 2 months ago

Merged, thanks!