[Q] Cleaning string whitespace trimming. Does it make any sense?

[Q] Cleaning string whitespace trimming. Does it make any sense?

[Ezequiel Garcia]

Hi Dan,

If you grep "isspace" through the code you will find lots of places
using, most of them
for removing leading and/or whitespace from a string.

Examples:

* sound/usb/card.c -> has a function of its own
http://tomoyo.sourceforge.jp/cgi-bin/lxr/ident?i=remove_trailing_spaces
* sound/pci/hda/hda_hwdep.c -> also! has a function of its own
http://tomoyo.sourceforge.jp/cgi-bin/lxr/ident?i=remove_trail_spaces
* drivers/char/ipmi/ipmi_si_intf.c -> embeds a custom algorithm ->
http://tomoyo.sourceforge.jp/cgi-bin/lxr/source/drivers/char/ipmi/ipmi_si_intf.c#L1690

As we have a special function for this (or could create in case there
isn't one already)
I wonder if it makes any sense to do some patches for this, since it
won't fix anything
and it's just to improve readability.

What do you think?

Thanks,
Ezequiel.

PD: Am I too pretty-code paranoid?

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Dan Carpenter]

We already have strim() which removes leading and trailing
whitespace.  But sure, that sounds like a good idea.  It would go in
front of strim() and strim would call it first then strim() would
look like this:

char *strim(char *s)
{
	trim_trailing(s);
	return skip_spaces(s);
}

Good luck on getting everyone to agree on a name for your function.
Python uses rstrip() for this.  CC Andrew Morton and lkml when you
have a patch ready.

regards,
dan carpenter

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Dan Carpenter]

Actually, on second thought strim() already does what we want.  Just
ignore the return value.

regards,
dan carpenter

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Ezequiel Garcia]

On Tue, Jun 12, 2012 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Actually, on second thought strim() already does what we want.  Just
> ignore the return value.
>

But it could make sense to fix such redundant code?

I'm unsure what git tree should I base the patches, given
I'd be touching all over the place. Perhaps linux-next?

Thanks,
Ezequiel.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Dan Carpenter]

On Tue, Jun 12, 2012 at 07:03:04PM -0300, Ezequiel Garcia wrote:
> On Tue, Jun 12, 2012 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Actually, on second thought strim() already does what we want.  Just
> > ignore the return value.
> >
> 
> But it could make sense to fix such redundant code?
> 

Yep.

> I'm unsure what git tree should I base the patches, given
> I'd be touching all over the place. Perhaps linux-next?

Yep.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[walter harms]

Am 13.06.2012 00:03, schrieb Ezequiel Garcia:
> On Tue, Jun 12, 2012 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>> Actually, on second thought strim() already does what we want.  Just
>> ignore the return value.
>>
> 
> But it could make sense to fix such redundant code?
> 
> I'm unsure what git tree should I base the patches, given
> I'd be touching all over the place. Perhaps linux-next?
> 

Hi Ezequiel,
as rule-of-the-thumb: just do it.

Convenience functions like trim() are added add irregular intervals because
they appear only if someone spots a pattern that he things is worth the effort.
Older Code does not know about it, so changes are required through the whole
code (look at patches from Julia Lawall, she has developed a tool to spot pattern).

How does it look from a maintainers perspective ?
1. He can remove a function from your code and reduce what you have to maintain -> good
2. He has a special need and need its own code -> but at least you know about it now :)

in short: you can make nothing wrong

re,
 wh

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Ezequiel Garcia]

On Wed, Jun 13, 2012 at 4:23 AM, walter harms <wharms@bfs.de> wrote:
Hi Walter,

>
> as rule-of-the-thumb: just do it.

Thanks for this advice. I'm gonna do just that.

>
> Convenience functions like trim() are added add irregular intervals because
> they appear only if someone spots a pattern that he things is worth the effort.
> Older Code does not know about it, so changes are required through the whole
> code (look at patches from Julia Lawall, she has developed a tool to spot pattern).
>
> How does it look from a maintainers perspective ?
> 1. He can remove a function from your code and reduce what you have to maintain -> good
> 2. He has a special need and need its own code -> but at least you know about it now :)
>
> in short: you can make nothing wrong

Okey, thanks again.

Regards,
Ezequiel.

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Ezequiel Garcia]

Dan,

On Tue, Jun 12, 2012 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Actually, on second thought strim() already does what we want.  Just
> ignore the return value.
>

If I *only* want to remove trailing whitespace, using strim() does the job
but it *also* tries to remove leading whitespace.

That can't hurt, but it also doesn't hurt to add a separate function
for that, right?

Thanks,
Ezequiel.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Dan Carpenter]

On Wed, Jun 13, 2012 at 03:16:24PM -0300, Ezequiel Garcia wrote:
> Dan,
> 
> On Tue, Jun 12, 2012 at 5:58 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > Actually, on second thought strim() already does what we want.  Just
> > ignore the return value.
> >
> 
> If I *only* want to remove trailing whitespace, using strim() does the job
> but it *also* tries to remove leading whitespace.
> 
> That can't hurt, but it also doesn't hurt to add a separate function
> for that, right?

It returns a pointer to the first non whitespace character, but you
can just ignore the return value.  Normally the first character is
going to be non whitespace, so it's not even a slow down.

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Ezequiel Garcia]

On Wed, Jun 13, 2012 at 3:21 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> It returns a pointer to the first non whitespace character, but you
> can just ignore the return value.  Normally the first character is
> going to be non whitespace, so it's not even a slow down.
>

Got it. I guess when you say there is no performance impact,
it's because this kind of functionality is used very sparsely in the code.
(Please correct me if I'm wrong.)

Thanks,
Ezequiel.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Q] Cleaning string whitespace trimming. Does it make any sense?

[Dan Carpenter]

On Wed, Jun 13, 2012 at 03:30:57PM -0300, Ezequiel Garcia wrote:
> On Wed, Jun 13, 2012 at 3:21 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > It returns a pointer to the first non whitespace character, but you
> > can just ignore the return value.  Normally the first character is
> > going to be non whitespace, so it's not even a slow down.
> >
> 
> Got it. I guess when you say there is no performance impact,
> it's because this kind of functionality is used very sparsely in the code.
> (Please correct me if I'm wrong.)

If the first character is non-white space then it's just one extra
branch statement.  And yeah, this tends to not be fast path code is
my guess.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html