On Fri, Jul 27, 2012 at 11:11 AM, Liu Yuan <namei.unix at gmail.com> wrote: > With a second review, I think this patch need more reviews. > > On 07/26/2012 09:46 PM, Yunkai Zhang wrote: >> +int send_light_req(struct sd_req *hdr, const char *host, int port) >> +{ >> + int fd, ret; >> + struct sd_rsp *rsp = (struct sd_rsp *)hdr; >> + unsigned rlen, wlen; >> + >> + fd = connect_to(host, port); >> + if (fd < 0) >> + return -1; >> + >> + rlen = 0; >> + wlen = 0; >> + ret = exec_req(fd, hdr, NULL, &wlen, &rlen); >> + close(fd); >> + >> + if (ret) { >> + return -1; >> + } > > Simply eprintf inside this function for connection failed > >> + >> + if (rsp->result != SD_RES_SUCCESS) { >> + eprintf("Response's result: %s\n", sd_strerror(rsp->result)); >> + return 1; >> + } >> + >> + return 0; >> +} > > Then use -1 for error case and 0 for success. No need to use both -1 and > 1 to represent failure case. The top code, such as cluster_shutdown/cluster_recover/..., need to use -1 and 1 to distinguish two error types: 1) failed to connect or failed to call exe_req() 2) success to call exe_re(), but the response's result isn't SD_RES_SUCCESS. > > Thanks, > Yuan > -- Yunkai Zhang Work at Taobao |