From owner-svn-src-all@FreeBSD.ORG Sun Apr 19 08:23:44 2015 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 40D7FF53; Sun, 19 Apr 2015 08:23:44 +0000 (UTC) Received: from mail110.syd.optusnet.com.au (mail110.syd.optusnet.com.au [211.29.132.97]) by mx1.freebsd.org (Postfix) with ESMTP id 779D7A38; Sun, 19 Apr 2015 08:23:43 +0000 (UTC) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail110.syd.optusnet.com.au (Postfix) with ESMTPS id B3F15780E07; Sun, 19 Apr 2015 18:23:27 +1000 (AEST) Date: Sun, 19 Apr 2015 18:23:26 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eitan Adler cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r281724 - head/usr.bin/rpcgen In-Reply-To: <201504190453.t3J4rTQT057322@svn.freebsd.org> Message-ID: <20150419172929.F1928@besplex.bde.org> References: <201504190453.t3J4rTQT057322@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=L/MkHYj8 c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=0J2RCfu4unCSrN9MOBsA:9 a=VqtkUkEFVafXuUsY:21 a=rwKKNUuyyiyYeohE:21 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 19 Apr 2015 08:23:44 -0000 On Sun, 19 Apr 2015, Eitan Adler wrote: > Log: > rpcgen: fix use use of strcmp > strcmp only guarantee that it will return at least 1 if the string B > is greater than that of string A. > > Modified: > head/usr.bin/rpcgen/rpc_sample.c > > Modified: head/usr.bin/rpcgen/rpc_sample.c > ============================================================================== > --- head/usr.bin/rpcgen/rpc_sample.c Sun Apr 19 04:27:50 2015 (r281723) > +++ head/usr.bin/rpcgen/rpc_sample.c Sun Apr 19 04:53:28 2015 (r281724) > @@ -115,7 +115,7 @@ write_sample_client(const char *program_ > for (l = proc->args.decls; l != NULL; l = l->next) { > f_print(fout, "\t"); > ptype(l->decl.prefix, l->decl.type, 1); > - if (strcmp(l->decl.type,"string") == 1) > + if (strcmp(l->decl.type,"string") >= 1) > f_print(fout, " "); > pvname(proc->proc_name, vp->vers_num); > f_print(fout, "_%s;\n", l->decl.name); It is still a bad example. strcmp is actually documented as returning > 0 if . ' >= 1' is just an obfuscated way of writing ' > 0'. Actually, it is still nonsense. I think l_decl.type is a keyword with a limited number of values. Probably 1 was never returned, since there is no keyword starting with "r" or "sr". So to avoid changing the behaviour, the correct change was to remove the dead code. However, the behaviour was probably broken. Signed comparison with keywords makes no sense. Only comparison for equality, or perhaps for being a prefix, makes sense. I think comparison for equality was intended. That could be written as 'strcmp ... == 0'. However, using strcmp to compare for equality would be a style bug. Everywhere else in the file, and almost everywhere else in the program spell this comparison using streq, since strcmp is apparently too hard to use. The other broken places are: - rpc_cout.c: 3 instances of strcmp, 9 of streq - rpc_main.c: 4 instances of strcmp, 4 of streq - rpc_parse.c: 2 instances of strcmp, 11 of streq - rpc_sample.c: 1 instances of strcmp, 6 of streq - rpc_svcout.c: 3 instances of strcmp, 5 of streq - rpc_util.c: 2 instances of strcmp, 9 of streq streq is used fairly consistently in old code, and the style was broken fairly consistently in changes (about half, including the above, apparently from Sun in 1995). There are often mounds of collateral style bugs near changes to use strcmp. In the above, the next statement is misindented. In this file, the indentation elsewhere is mostly using tabs. It is never using spaces where that would be correct, but is often where it is incorrect (mostly for corrupted tabs and 2-space indentation). Old sun code had many more indentation errors from 5-space indentation for K&R function headers. Altogether, this file had a mixture of 2-space indentation, 4-space indentation which gave bogus lining up of parentheses (in the above code), 5-space indentation, correct lining-up-parentheses indentation, 8-space indentation from corrupt tabs, and normal indentation with tabs. A grate sample. Bruce