On Mon, Sep 18, 2017 at 12:27 PM, Baoquan He <bhe(a)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(a)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_FILE
> >> > + return 1
> >>
> >> How 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