Date: Fri, 05 Sep 2014 16:56:21 +0400 From: "Alexander V. Chernikov" <melifaro@FreeBSD.org> To: Steven Hartland <killing@multiplay.co.uk>, src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: Re: svn commit: r271160 - projects/ipfw/sbin/ipfw Message-ID: <5409B2F5.8080500@FreeBSD.org> In-Reply-To: <AD38D8610FF24B7AABCF81E5C6DF10CC@multiplay.co.uk> References: <201409051148.s85BmX9Y066331@svn.freebsd.org> <AD38D8610FF24B7AABCF81E5C6DF10CC@multiplay.co.uk>
index | next in thread | previous in thread | raw e-mail
On 05.09.2014 16:49, Steven Hartland wrote: > Why not eliminate error in do_set3(..) and just: > return (setsockopt(ipfw_socket, IPPROTO_IP, IP_FW3, op3, optlen)); This makes things more abstract/portable. Other callers know nothing about exact implementation (ipfw_socket & IPPROTO_IP). ipfw over netlink (or userland implementation) can be good examples where this might be needed. > > ----- Original Message ----- From: "Alexander V. Chernikov" > <melifaro@FreeBSD.org> > To: <src-committers@freebsd.org>; <svn-src-projects@freebsd.org> > Sent: Friday, September 05, 2014 12:48 PM > Subject: svn commit: r271160 - projects/ipfw/sbin/ipfw > > >> Author: melifaro >> Date: Fri Sep 5 11:48:32 2014 >> New Revision: 271160 >> URL: http://svnweb.freebsd.org/changeset/base/271160 >> >> Log: >> Use per-function errno handling instead of global one. >> >> Requested by: luigi >> >> Modified: >> projects/ipfw/sbin/ipfw/ipfw2.c >> projects/ipfw/sbin/ipfw/tables.c >> >> Modified: projects/ipfw/sbin/ipfw/ipfw2.c >> ============================================================================== >> >> --- projects/ipfw/sbin/ipfw/ipfw2.c Fri Sep 5 11:25:58 2014 (r271159) >> +++ projects/ipfw/sbin/ipfw/ipfw2.c Fri Sep 5 11:48:32 2014 (r271160) >> @@ -575,7 +575,7 @@ do_cmd(int optname, void *optval, uintpt >> int >> do_set3(int optname, ip_fw3_opheader *op3, uintptr_t optlen) >> { >> - int errno; >> + int error; >> >> if (co.test_only) >> return (0); >> @@ -587,10 +587,9 @@ do_set3(int optname, ip_fw3_opheader *op >> >> op3->opcode = optname; >> >> - if (setsockopt(ipfw_socket, IPPROTO_IP, IP_FW3, op3, optlen) != 0) >> - return (errno); >> + error = setsockopt(ipfw_socket, IPPROTO_IP, IP_FW3, op3, optlen); >> >> - return (0); >> + return (error); >> } >> >> /* >> @@ -621,11 +620,6 @@ do_get3(int optname, ip_fw3_opheader *op >> error = getsockopt(ipfw_socket, IPPROTO_IP, IP_FW3, op3, >> (socklen_t *)optlen); >> >> - if (error == -1) { >> - if (errno != 0) >> - error = errno; >> - } >> - >> return (error); >> } >> >> @@ -2511,7 +2505,7 @@ ipfw_list(int ac, char *av[], int show_c >> sfo.flags |= IPFW_CFG_GET_STATES; >> if (sfo.show_counters != 0) >> sfo.flags |= IPFW_CFG_GET_COUNTERS; >> - if ((error = ipfw_get_config(&co, &sfo, &cfg, &sz)) != 0) >> + if (ipfw_get_config(&co, &sfo, &cfg, &sz) != 0) >> err(EX_OSERR, "retrieving config failed"); >> >> error = ipfw_show_config(&co, &sfo, cfg, sz, ac, av); >> @@ -2654,7 +2648,7 @@ ipfw_get_config(struct cmdline_opts *co, >> { >> ipfw_cfg_lheader *cfg; >> size_t sz; >> - int error, i; >> + int i; >> >> >> if (co->test_only != 0) { >> @@ -2676,10 +2670,10 @@ ipfw_get_config(struct cmdline_opts *co, >> cfg->start_rule = fo->first; >> cfg->end_rule = fo->last; >> >> - if ((error = do_get3(IP_FW_XGET, &cfg->opheader, &sz)) != 0) { >> - if (error != ENOMEM) { >> + if (do_get3(IP_FW_XGET, &cfg->opheader, &sz) != 0) { >> + if (errno != ENOMEM) { >> free(cfg); >> - return (error); >> + return (errno); >> } >> >> /* Buffer size is not enough. Try to increase */ >> @@ -4865,23 +4859,23 @@ ipfw_get_tracked_ifaces(ipfw_obj_lheader >> { >> ipfw_obj_lheader req, *olh; >> size_t sz; >> - int error; >> >> memset(&req, 0, sizeof(req)); >> sz = sizeof(req); >> >> - error = do_get3(IP_FW_XIFLIST, &req.opheader, &sz); >> - if (error != 0 && error != ENOMEM) >> - return (error); >> + if (do_get3(IP_FW_XIFLIST, &olh->opheader, &sz) != 0) { >> + if (errno != ENOMEM) >> + return (errno); >> + } >> >> sz = req.size; >> if ((olh = calloc(1, sz)) == NULL) >> return (ENOMEM); >> >> olh->size = sz; >> - if ((error = do_get3(IP_FW_XIFLIST, &olh->opheader, &sz)) != 0) { >> + if (do_get3(IP_FW_XIFLIST, &olh->opheader, &sz) != 0) { >> free(olh); >> - return (error); >> + return (errno); >> } >> >> *polh = olh; >> >> Modified: projects/ipfw/sbin/ipfw/tables.c >> ============================================================================== >> >> --- projects/ipfw/sbin/ipfw/tables.c Fri Sep 5 11:25:58 2014 (r271159) >> +++ projects/ipfw/sbin/ipfw/tables.c Fri Sep 5 11:48:32 2014 (r271160) >> @@ -497,7 +497,7 @@ static void >> table_modify(ipfw_obj_header *oh, int ac, char *av[]) >> { >> ipfw_xtable_info xi; >> - int error, tcmd; >> + int tcmd; >> size_t sz; >> char tbuf[128]; >> >> @@ -520,7 +520,7 @@ table_modify(ipfw_obj_header *oh, int ac >> } >> } >> >> - if ((error = table_do_modify(oh, &xi)) != 0) >> + if (table_do_modify(oh, &xi) != 0) >> err(EX_OSERR, "Table modification failed"); >> } >> >> @@ -553,14 +553,13 @@ static void >> table_lock(ipfw_obj_header *oh, int lock) >> { >> ipfw_xtable_info xi; >> - int error; >> >> memset(&xi, 0, sizeof(xi)); >> >> xi.mflags |= IPFW_TMFLAGS_LOCK; >> xi.flags |= (lock != 0) ? IPFW_TGFLAGS_LOCKED : 0; >> >> - if ((error = table_do_modify(oh, &xi)) != 0) >> + if (table_do_modify(oh, &xi) != 0) >> err(EX_OSERR, "Table %s failed", lock != 0 ? "lock" : "unlock"); >> } >> >> @@ -641,7 +640,6 @@ static int >> table_get_info(ipfw_obj_header *oh, ipfw_xtable_info *i) >> { >> char tbuf[sizeof(ipfw_obj_header) + sizeof(ipfw_xtable_info)]; >> - int error; >> size_t sz; >> >> sz = sizeof(tbuf); >> @@ -649,8 +647,8 @@ table_get_info(ipfw_obj_header *oh, ipfw >> memcpy(tbuf, oh, sizeof(*oh)); >> oh = (ipfw_obj_header *)tbuf; >> >> - if ((error = do_get3(IP_FW_TABLE_XINFO, &oh->opheader, &sz)) != 0) >> - return (error); >> + if (do_get3(IP_FW_TABLE_XINFO, &oh->opheader, &sz) != 0) >> + return (errno); >> >> if (sz < sizeof(tbuf)) >> return (EINVAL); >> @@ -1058,7 +1056,6 @@ table_do_lookup(ipfw_obj_header *oh, cha >> ipfw_obj_tentry *tent; >> uint8_t type; >> uint32_t vmask; >> - int error; >> size_t sz; >> >> memcpy(xbuf, oh, sizeof(*oh)); >> @@ -1073,8 +1070,8 @@ table_do_lookup(ipfw_obj_header *oh, cha >> oh->ntlv.type = type; >> >> sz = sizeof(xbuf); >> - if ((error = do_get3(IP_FW_TABLE_XFIND, &oh->opheader, &sz)) != 0) >> - return (error); >> + if (do_get3(IP_FW_TABLE_XFIND, &oh->opheader, &sz) != 0) >> + return (errno); >> >> if (sz < sizeof(xbuf)) >> return (EINVAL); >> @@ -1556,14 +1553,13 @@ tables_foreach(table_cb_t *f, void *arg, >> return (ENOMEM); >> >> olh->size = sz; >> - error = do_get3(IP_FW_TABLES_XLIST, &olh->opheader, &sz); >> - if (error == ENOMEM) { >> - sz = olh->size; >> - free(olh); >> - continue; >> - } else if (error != 0) { >> + if (do_get3(IP_FW_TABLES_XLIST, &olh->opheader, &sz) != 0) { >> free(olh); >> - return (error); >> + if (errno == ENOMEM) { >> + sz = olh->size; >> + continue; >> + } >> + return (errno); >> } >> >> if (sort != 0) >> @@ -1595,11 +1591,10 @@ table_do_get_list(ipfw_xtable_info *i, i >> { >> ipfw_obj_header *oh; >> size_t sz; >> - int error, c; >> + int c; >> >> sz = 0; >> oh = NULL; >> - error = 0; >> for (c = 0; c < 8; c++) { >> if (sz < i->size) >> sz = i->size + 44; >> @@ -1609,19 +1604,17 @@ table_do_get_list(ipfw_xtable_info *i, i >> continue; >> table_fill_objheader(oh, i); >> oh->opheader.version = 1; /* Current version */ >> - error = do_get3(IP_FW_TABLE_XLIST, &oh->opheader, &sz); >> - >> - if (error == 0) { >> + if (do_get3(IP_FW_TABLE_XLIST, &oh->opheader, &sz) == 0) { >> *poh = oh; >> return (0); >> } >> >> - if (error != ENOMEM) >> + if (errno != ENOMEM) >> break; >> } >> free(oh); >> >> - return (error); >> + return (errno); >> } >> >> /* >> @@ -1798,23 +1791,22 @@ table_do_get_stdlist(uint16_t opcode, ip >> { >> ipfw_obj_lheader req, *olh; >> size_t sz; >> - int error; >> >> memset(&req, 0, sizeof(req)); >> sz = sizeof(req); >> >> - error = do_get3(opcode, &req.opheader, &sz); >> - if (error != 0 && error != ENOMEM) >> - return (error); >> + if (do_get3(opcode, &req.opheader, &sz) != 0) >> + if (errno != ENOMEM) >> + return (errno); >> >> sz = req.size; >> if ((olh = calloc(1, sz)) == NULL) >> return (ENOMEM); >> >> olh->size = sz; >> - if ((error = do_get3(opcode, &olh->opheader, &sz)) != 0) { >> + if (do_get3(opcode, &olh->opheader, &sz) != 0) { >> free(olh); >> - return (error); >> + return (errno); >> } >> >> *polh = olh; >> >> >help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?5409B2F5.8080500>
