On Mon, Sep 18, 2017 at 12:27 PM, Baoquan He bhe@redhat.com wrote:
On 09/18/17 at 12:17pm, Bhupesh Sharma wrote:
Hi Bao,
Thanks for the review.
On Mon, Sep 18, 2017 at 6:25 AM, Baoquan He bhe@redhat.com wrote:
On 09/18/17 at 08:34am, Baoquan He wrote:
On 09/17/17 at 09:28pm, Bhupesh Sharma wrote:
- # Check if we have any leading spaces (or tabs) before the
- # variable name in the kdump conf file. While doing so exclude any
- # comments which appear to have leading spaces (or tabs)
- awk 'BEGIN{cnt=0} {cnt=match($0, /^[[:blank:]]/); if(cnt!=0) print $0}' $KDUMP_CONFIG_FILE >> $TMP_FILE
- nr=$(awk 'BEGIN{cnt_hash=0; cnt_blank=0} {cnt_hash=match($0, /#.*$/); if(cnt_hash!=0) next; else cnt_blank++} END{print cnt_blank}' $TMP_FILE)
- [ $nr -gt 0 ] && {
echo "No whitespaces are allowed before a kdump option name. Please check $KDUMP_CONFIG_FILE"rm -f $TMP_FILEreturn 1How about this:
if grep -v "^[[:blank:]]*#" /etc/kdump.conf| grep -v "^$"| grep "^[[:space:]]"; then fi
It can ignore the comment at the beginning with [[:blank:]], and blank line, only check line with config info.
And why don't we just help erase the [[:blank:]] at the beginning of config line, like this:
grep -v "^[[:blank:]]*#" /etc/kdump.conf| grep -v "^$"| grep -v "^[[:space:]]"
OK, forget this saying. Here it's just checking. So the command 'read' won't filter out the [[:blank:]] for config and value?
This was discussed in the original BZ report: https://bugzilla.redhat.com/show_bug.cgi?id=1484945
i.e.,
- We can either say that there should be no space before the path by
failing the service, or
- We will have to change the code and remove the spaces in /etc/kdump.conf file.
The 2nd option looks better to me if we can fix it. Of course we can also give a warning if agreement has been made by you and Dave. But using awk to check that is a little overkilling.
Personal opinion.
Yes, we discussed the above and agreed on the former.
However, I am not too specific about any of the same and would need inputs from Dave and others also to decide how to go about the same.
Regarding grep v/s awk, I think we can chose either one of the same. v2 used grep but did not handle the space before a comment use case and hence I decided to use the awk in v3 as it is more compact for this particular search.
Regards, Bhupesh
However, as discussed with Dave on irc earlier, we need to add proper error handling in kdumpctl, as any user can add a space before a variable in kdump.conf and this should be handled properly.
This makes us go with the option: There should be no space before the path by failing the service.
Kindly let me know if I have missed anything.
@Dave: Kindly share your inputs as well.
Regards, Bhupesh