> On Jan. 2, 2017, 5:58 p.m., Don Bosco Durai wrote: > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java, > > lines 312-314 > > <https://reviews.apache.org/r/55008/diff/1/?file=1591529#file1591529line312> > > > > I agree with Alok, blocks without braces are like disastor waiting to > > happen. It would be good to enclose within brackets > > Qiang Zhang wrote: > At the beginning of the getIntConfig I have set a default value for the > ret. > int ret = defaultValue; > The following logic can be executed when the retStr is null. > else > ret = defaultValue; > Since we have default the value for ret, so there is no need to execute > the logic. > So we don't need to enclose within brackets.
Hi Qiang, the logic is correct and also the syntax is valid. The feedback is just regarding the curly braces. Later on, if someone adds another line for the same if condition, but not put the braces, then it would have some unexpected consequences. Having the braces is just a good practice even for one line statements and is followed by many. - Don Bosco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55008/#review160334 ----------------------------------------------------------- On Dec. 23, 2016, 6:54 a.m., Qiang Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55008/ > ----------------------------------------------------------- > > (Updated Dec. 23, 2016, 6:54 a.m.) > > > Review request for ranger, Don Bosco Durai, Madhan Reddy, Ramesh Mani, > Selvamohan Neethiraj, and Velmurugan Periasamy. > > > Bugs: RANGER-1280 > https://issues.apache.org/jira/browse/RANGER-1280 > > > Repository: ranger > > > Description > ------- > > I failed to run the ranger-admin command and couldn't find any error messages > in log file after set > ranger.service.http.connector.attrib.maxHeaderCount=aa(string value) in > ranger-admin-site.xml file. I analyzed the source codes and found the reason. > I found there are two questions after check the source codes in > EmbeddedServer.java. > 1. No exception pop out when called Integer.parseInt() and Long.parseLong() > function. Once abnormal, the program aborted directly without log. > 2. The catch captures anomaly without log in the loadConfig function. It only > calls e.printStackTrace() function. > > Currently the program will be aborted when the exception occured. We should > get the default value instead of aborted. > > We have tested our patch strictly. > > > Diffs > ----- > > > embeddedwebserver/src/main/java/org/apache/ranger/server/tomcat/EmbeddedServer.java > 7ebba8a > > Diff: https://reviews.apache.org/r/55008/diff/ > > > Testing > ------- > > > Thanks, > > Qiang Zhang > >
