[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] sysinfo: delete unnecessary white space of sysinfo.



Hi, Eric

Thank you for your review.

On Mon, 27 Jun 2011 09:03:57 -0600
Eric Blake <eblake redhat com> wrote:

> On 06/27/2011 04:13 AM, Daniel P. Berrange wrote:
> > On Mon, Jun 27, 2011 at 05:25:17PM +0900, Minoru Usui wrote:
> >> sysinfo: delete unnecessary white space of sysinfo.
> >>
> >>   * Add virSkipSpacesBackwards() to src/util/util.[ch]
> >>   * Trim each element and delete null entry of sysinfo by virSkipSpacesBackwards().
> >>
> >> Signed-off-by: Minoru Usui <usui mxm nes nec co jp>
> >> ---
> >>  src/util/sysinfo.c |   34 +++++++++++++++++++++++++---------
> >>  src/util/util.c    |   27 +++++++++++++++++++++++++++
> >>  src/util/util.h    |    1 +
> >>  3 files changed, 53 insertions(+), 9 deletions(-)
> >> +void
> >> +virSkipSpacesBackwards(const char *str, char **endp)
> >> +{
> >> +    char *cur;
> >> +
> >> +    if (!endp || !*endp)
> >> +        return;
> >> +
> >> +    cur = *endp - 1;
> >> +    while (cur >= str) {
> >> +        if ((*cur == ' ') || (*cur == '\t') || (*cur == '\n') ||
> >> +            (*cur == '\r') || (*cur == '\\'))
> 
> Maybe we should use c_isspace() instead of open-coding this.  And how is
> backslash a space?

I'll replace above block to c_isspace().

BTW, I copied its block from virSkipSpaces().
It seems meaningful virSkipSpaces() treats backslash as space,
but I think backslash is not space generally.

Is it OK that virSkipSpaces() treats backslash as space?
Doesn't it need to rename something and define another virSkipSpaces()
which doesn't treat backslash as space?
-- 
Minoru Usui <usui mxm nes nec co jp>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]