Re: [PATCH] Nitpick to libsvn_subr/svn_string.c

Quoting Branko ??ibej (brane@xbc.nu):
> Please please read the section in HACKING about writing log messages.

Bleh, sorry for my ignorance.

Indentation fixed,
Diff reduced against trunk by insterting and removing newlines where
  there were differences,
Space inserted after function identifier, before () group,
__ removed from code,
static, APR_INLINE where applicable,
first_non_whitespace*, and string_compare* functions common logic
  hoisted into helper functions.
More tests for all changed functions,
Correct log message (hopefully).

This time I tried to do what you said, log message and patch follows,
maybe it needs another round, but we are definitely getting much
closer.

--- Log message
Cleanup of subversion/libsvn_subr/svn_string.c.

* subversion/libsvn_subr/svn_string.c
  (string_compare): New function with common code for comparing
    strings. No change in existing code.
  (string_first_non_whitespace): New function with common code for
    finding the first character which is not a whitespace character.
    No change in existing code.
  (find_char_backward): New function with common code for finding
    a given character in the string, searching backwards. Eliminates
    a signedness issue by using an unsigned counter and counting
    downwards in the for-loop.
  (svn_string_compare, svn_stringbuf_compare,
   svn_string_compare_stringbuf): Use the new common function
    string_compare for these 3 functions.
  (svn_string_first_non_whitespace,
   svn_stringbuf_first_non_whitespace): Use the new common function
    string_first_non_whitespace for these 2 functions.
  (svn_string_find_char_backward,
   svn_stringbuf_find_char_backward): Use the new common function
    find_char_backward for these 2 functions.
  (svn_stringbuf_ensure): Add a comment to the place where the code
    checks for overflow of an apr_size_t to help readers of the code.

* subversion/tests/libsvn_subr/string-test.c
  (test_find_char_backward): New function, which calls
    svn_stringbuf_find_char_backward on a string and checks if the
    position the character was found matches the expected value.
  (test13): Using test_find_char_backward, test the case where
    the character is in the middle of the string.
  (test14): Using test_find_char_backward, test the case where
    the character is in the very beginning of the string.
  (test15): Using test_find_char_backward, test the case where
    the character is in the end of the string.
  (test16): Using test_find_char_backward, test the case where
    the string has length 0.
  (test17): Using test_find_char_backward, test the case where
    the string doesn't contain the character we searched for.
  (test_first_non_whitespace): New function. Given a string and an
    expected position for the first non-whitespace character, check
    if the svn_stringbuf_first_non_whitespace agrees.
  (test18): Using test_first_non_whitespace, check the common case
    where there is whitespace.
  (test19): Using test_first_non_whitespace, check the case where
    there is no whitespace.
  (test20): Using test_first_non_whitespace, check the case where
    all of the string is whitespace.
  (test21): Check the correctness of svn_stringbuf_strip_whitespace
    in the common case.
  (test_string_compare): New function. Given 2 strings, creates
    stringbuffers for these and compares them via
    svn_stringbuf_compare, returning the outcome.
  (test22): Check that svn_stringbuf_compare fails on inputs with
    different lengths, by using test_stringbuf_compare.
  (test23): Check that svn_stringbuf_compare fails on inputs of
    equal length, but different content, by using
    test_stringbuf_compare.
  (test_funcs[]): Updated with new tests.

---
-- 
jlouis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org