On Thu, Nov 13, 2014 at 01:56:17PM -0600, Mark Tinguely wrote:
> On 11/13/14 13:41, Eric Sandeen wrote:
> >On 11/13/14 1:14 PM, Mark Tinguely wrote:
> >>Linux strcpy() corrupts the output string when the input
> >
> >Not Linux strcpy in particular; per C99:
> >
> >>The strcpy function copies the string pointed to by s2
> >>(including the terminating null character) into the array
> >>pointed to by s1. If copying takes place between objects
> >>that overlap, the behavior is undefined.
> > ^^^^^^^^^^^^^^^^^^^^^^^^^
> >
> >>and output strings overlap. The shrink() function in xfsrestore
> >>uses an overlapping strcpy() to remove special characters when
> >>processing an interactive command line. The resultant command
> >>will fail.
> >>
> >>examples:
> >> -> cd "AOGC exome chip core genotyping"
> >>AOGC exome chp core genotyping not found
> >> -> cd "t t"
> >>tt not found
> >>
> >>Fix my manually moving the characters in the array.
> >>
> >>Signed-off-by: Mark Tinguely<tinguely@xxxxxxx>
> >>---
> >> restore/tree.c | 14 +++++++++++++-
> >> 1 file changed, 13 insertions(+), 1 deletion(-)
> >>
> >>Index: b/restore/tree.c
> >>===================================================================
> >>--- a/restore/tree.c
> >>+++ b/restore/tree.c
> >>@@ -4857,7 +4857,19 @@ distance_to_space( char *s, char *l )
> >> static void
> >> shrink( char *s, size_t cnt )
> >> {
> >>- strcpy( s, s + cnt );
> >>+ /*
> >>+ * Linux strcpy corrupts the string if the src and dst overlap.
> >>+ * Manually copy the entries to the left.
> >>+ *
> >>+ * Since the liter array is mostly nulls, shrink is not moving
> >
> >what is the "liter array?" Ah well. Context. ;)
> >
> >>+ * the array left as intended. Does not seem to be many embedded
> >>+ * processing characters, so leaving it for now
> >>+ */
> >>+ char *m = s + cnt;
> >>+ while (*m != '\0')
> >>+ *s++ = *m++;
> >>+ /* NULL the last character of the string */
> >>+ *s = '\0';
> >> }
> >
> >Would this be any less manual?
> >
> > size_t n = strlen(s+cnt) + 1; /* 1 for terminating NULL */
> >
> > memmove(s, s + cnt, n);
> >
> >because memmove is ok with overlaps.
> >
> >-Eric
> >
>
> I thought of that but if we are doing a strlen() might as well just
> copy it while you are walking the string.
I'd much prefer the strlen() + memmove() because then it's self
documenting that the move of the string is overlapping. It will also
be faster for large strings than a manual byte-at-a-time copy,
and there's less code to maintain overall. ;)
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
|