Re: [PATCH] Improve function dir.c:trim_trailing_spaces()

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

 



On Wed, May 28, 2014 at 04:45:57PM -0700, Pasha Bolokhov wrote:

> Move backwards from the end of the string (more efficient for
> lines which do not have trailing spaces or have just a couple).

The original code reads the string from left to right. In theory, that
means we could get away with not calling strlen() at all, over a
right-to-left that must start with a call to strlen().

That being said, I think we already have the length in the calling
function, so you could probably avoid the strlen() altogether, which
makes a right-to-left function more efficient.

However, I doubt it makes that much of a difference in practice, so
unless it's measurable, I would certainly go with the version that is
more readable (and correct, of course).

> Slightly more rare occurrences of 'text  \    ' with a backslash
> in between spaces are handled correctly.

Can you add a test for this?

Also, if you are refactoring this function, I think it makes sense to
check that:

  "foo\\ "

and

  "foo\\\ "

match "foo\" and "foo\ ", respectively (I think they do with your patch,
but it is a tricky case that is not immediately obvious).

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]