| Message ID | 1426453252-30326-1-git-send-email-luka@openwrt.org |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi, On 15/03/2015 22:00, Luka Perkov wrote: > wbuf = blobmsg_alloc_string_buffer(&buf, "md5", 33); > + > + for (i = 0; i < 16; i++) > + sprintf((wbuf + (i * 2)), "%02x", (uint8_t) md5[i]); there is a set of brackets too many here > + > + *(wbuf + 33) = 0; is this not off by 1 ? if we want to be pedantic it should also be '\0' i think wbuf[32] = '\0'; is a saner syntax John
Hi John, On Sun, Mar 15, 2015 at 10:21:27PM +0100, John Crispin wrote: > On 15/03/2015 22:00, Luka Perkov wrote: > > wbuf = blobmsg_alloc_string_buffer(&buf, "md5", 33); > > + > > + for (i = 0; i < 16; i++) > > + sprintf((wbuf + (i * 2)), "%02x", (uint8_t) md5[i]); > > there is a set of brackets too many here Ok. I've fixed it locally. > > + *(wbuf + 33) = 0; > > is this not off by 1 ? No. > if we want to be pedantic it should also be '\0' > > i think wbuf[32] = '\0'; is a saner syntax Ok. I've queued this for v2 which I will send after libubox base64 patch. Luka
Hi, On Mon, Mar 16, 2015 at 6:47 PM, Luka Perkov <luka@openwrt.org> wrote: > Hi John, > > On Sun, Mar 15, 2015 at 10:21:27PM +0100, John Crispin wrote: >> On 15/03/2015 22:00, Luka Perkov wrote: >> > wbuf = blobmsg_alloc_string_buffer(&buf, "md5", 33); >> > + >> > + for (i = 0; i < 16; i++) >> > + sprintf((wbuf + (i * 2)), "%02x", (uint8_t) md5[i]); >> >> there is a set of brackets too many here > > Ok. I've fixed it locally. > >> > + *(wbuf + 33) = 0; >> >> is this not off by 1 ? > > No. > >> if we want to be pedantic it should also be '\0' >> >> i think wbuf[32] = '\0'; is a saner syntax > > Ok. I've queued this for v2 which I will send after libubox base64 > patch. sprintf null-terminates the generated string, so the 16th sprintf() will already have set wbuf[33] to \0. Jonas
On 16/03/2015 18:47, Luka Perkov wrote: >>> + *(wbuf + 33) = 0; >>> >>> is this not off by 1 ? > No. > ?!?! you are accessing the memory behind wbuf. my question was a statement wrapped as a question to be polite. let me rephrase this is a definate off by one. please fix it.
diff --git a/file.c b/file.c index 31a937d..9e87a10 100644 --- a/file.c +++ b/file.c @@ -27,6 +27,7 @@ #include <sys/wait.h> #include <libubus.h> #include <libubox/blobmsg.h> +#include <libubox/md5.h> #include <libubox/ustream.h> #include <rpcd/plugin.h> @@ -237,6 +238,41 @@ rpc_file_write(struct ubus_context *ctx, struct ubus_object *obj, } static int +rpc_file_md5(struct ubus_context *ctx, struct ubus_object *obj, + struct ubus_request_data *req, const char *method, + struct blob_attr *msg) +{ + int rv, i; + char *path; + struct stat s; + uint8_t md5[16]; + char *wbuf; + + if (!rpc_check_path(msg, &path, &s)) + return rpc_errno_status(); + + if (!S_ISREG(s.st_mode)) + return UBUS_STATUS_NOT_SUPPORTED; + + if ((rv = md5sum(path, md5)) <= 0) + return rpc_errno_status(); + + blob_buf_init(&buf, 0); + wbuf = blobmsg_alloc_string_buffer(&buf, "md5", 33); + + for (i = 0; i < 16; i++) + sprintf((wbuf + (i * 2)), "%02x", (uint8_t) md5[i]); + + *(wbuf + 33) = 0; + + blobmsg_add_string_buffer(&buf); + ubus_send_reply(ctx, req, buf.head); + blob_buf_free(&buf); + + return UBUS_STATUS_OK; +} + +static int rpc_file_list(struct ubus_context *ctx, struct ubus_object *obj, struct ubus_request_data *req, const char *method, struct blob_attr *msg) @@ -611,6 +647,7 @@ rpc_file_api_init(const struct rpc_daemon_ops *o, struct ubus_context *ctx) UBUS_METHOD("write", rpc_file_write, rpc_file_rw_policy), UBUS_METHOD("list", rpc_file_list, rpc_file_r_policy), UBUS_METHOD("stat", rpc_file_stat, rpc_file_r_policy), + UBUS_METHOD("md5", rpc_file_md5, rpc_file_r_policy), UBUS_METHOD("exec", rpc_file_exec, rpc_exec_policy), };
Signed-off-by: Luka Perkov <luka@openwrt.org> --- file.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)