https://bugzilla.mindrot.org/show_bug.cgi?id=2948
--- Comment #9 from Damien Miller <[email protected]> --- Comment on attachment 3344 --> https://bugzilla.mindrot.org/attachment.cgi?id=3344 sftp server copy-data extension looks good - some minor comments >diff --git a/PROTOCOL b/PROTOCOL >index f75c1c0ae5b0..04a392db33be 100644 ... >+static void >+process_extended_copy_data(u_int32_t id) ... >+ /* Disallow reading & writing to the same handle */ >+ if (read_handle == write_handle || read_fd < 0 || write_fd < 0) { I think this should also check that both the read and write handles do not refer to the same path? (use handle_to_name()) >+ status = SSH2_FX_FAILURE; >+ } else { nit: prefer "goto out" over nesting if/else >+ if (lseek(read_fd, read_off, SEEK_SET) < 0) { >+ status = errno_to_portable(errno); >+ error("process_extended_copy_data: read_seek failed"); nit: ditto >+ } else if (!(handle_to_flags(write_handle) & O_APPEND) && >+ lseek(write_fd, write_off, SEEK_SET) < 0) { >+ status = errno_to_portable(errno); >+ error("process_extended_copy_data: write_seek failed"); nit: prefer __func__ to manual inclusion of function name >+ } else { >+ /* Process the request in chunks. */ >+ while (read_len || copy_until_eof) { nit: prefer explicit comparison against zero (i.e "read_len > 0") >+ >+ ret = read(read_fd, buf, len); ... >+ ret = write(write_fd, buf, len); I think this should use atomicio here to be signal safe. >+ if ((size_t)ret != len) { >+ debug2("nothing at all written"); >+ status = SSH2_FX_FAILURE; >+ break; >+ } this block can go away with atomicio -- You are receiving this mail because: You are watching the assignee of the bug. You are watching someone on the CC list of the bug. _______________________________________________ openssh-bugs mailing list [email protected] https://lists.mindrot.org/mailman/listinfo/openssh-bugs
