> The only difference between translate_newline() and translate_newline_alt()
> is the lines of code from here to the end of the function. I think you
> could keep translate_newline() as is, and just move these checks elsewhere
> (to a new function or to the caller; I haven't thought about this in
> detail).
>
> In any case, please avoid the code duplication if possible. (Function
> pointers are nice, but sometimes just a way to shoot yourself in the
> foot.) If you're unsure what approach to take, do brainstorm on-list
> before sending a new iteration of the patch. (That saves time to you
> and to us)
After thinking about this problem some more, I thought about using a
macro solution. I define a macro that is able to output the
definitions of the two variants of translate_newline():
translate_newline() and translate_newline_alt(). This way, duplicate
code is avoided, but it is like having duplicate code where each copy
is slightly different.
Do you like this idea? I have attached C code that I have tested. In
this sample, the macro that outputs the definitions of
translate_newline() and translate_newline_alt() is called
DEFINE_TRANSLATE_NEWLINE_FN.
/* A boolean expression that evaluates to true if the string STR, having length
STR_LEN, is one of the end-of-line strings LF, CR, or CRLF; to false
otherwise. */
#define STRING_IS_EOL(str, str_len) (((str_len) == 2 && \
(str)[0] == '\r' && \
(str)[1] == '\n') || \
((str_len) == 1 && \
((str)[0] == '\n' || (str)[0] == '\r')))
/* A boolean expression that evaluates to true if the end-of-line string EOL1,
having length EOL1_LEN, and the end-of-line string EOL2, having length
EOL2_LEN, are different, assuming that EOL1 and EOL2 are both from the
set {"\n", "\r", "\r\n"}; to false otherwise.
Given that EOL1 and EOL2 are either "\n", "\r", or "\r\n", then if
EOL1_LEN is not the same as EOL2_LEN, then EOL1 and EOL2 are of course
different. If EOL1_LEN and EOL2_LEN are both 2 then EOL1 and EOL2 are both
"\r\n". Otherwise, EOL1_LEN and EOL2_LEN are both 1. We need only check the
one character for equality to determine whether EOL1 and EOL2 are the same
in that case. */
#define DIFFERENT_EOL_STRINGS(eol1, eol1_len, eol2, eol2_len) \
(((eol1_len) != (eol2_len)) || (*(eol1) != *(eol2)))
static svn_error_t *
translate_newline_alt(const char *eol_str,
apr_size_t eol_str_len,
char *src_format,
apr_size_t *src_format_len,
const char *newline_buf,
apr_size_t newline_len,
svn_stream_t *dst,
struct translation_baton *b);
#define DEFINE_TRANSLATE_NEWLINE_FN(fn, is_alt) \
static svn_error_t * \
fn(const char *eol_str, \
apr_size_t eol_str_len, \
char *src_format, \
apr_size_t *src_format_len, \
const char *newline_buf, \
apr_size_t newline_len, \
svn_stream_t *dst, \
struct translation_baton *b) \
{ \
SVN_ERR_ASSERT(STRING_IS_EOL(eol_str, eol_str_len)); \
SVN_ERR_ASSERT(STRING_IS_EOL(newline_buf, newline_len)); \
\
/* If we've seen a newline before, compare it with our cache to \
check for consistency, else cache it for future comparisons. */ \
if (*src_format_len) \
{ \
/* Comparing with cache. If we are inconsistent and \
we are NOT repairing the file, generate an error! */ \
if ((! b->repair) && \
((*src_format_len != newline_len) || \
(strncmp(src_format, newline_buf, newline_len)))) \
return svn_error_create(SVN_ERR_IO_INCONSISTENT_EOL, NULL, NULL); \
} \
else \
{ \
/* This is our first line ending, so cache it before \
handling it. */ \
strncpy(src_format, newline_buf, newline_len); \
*src_format_len = newline_len; \
} \
\
/* Translate the newline */ \
SVN_ERR(translate_write(dst, eol_str, eol_str_len)); \
\
if (!(is_alt)) \
{ \
/* Set *b->translated_eol to TRUE if a different line ending string was \
written out. */ \
SVN_ERR_ASSERT(b->translated_eol != NULL); \
if (DIFFERENT_EOL_STRINGS(eol_str, eol_str_len, \
newline_buf, newline_len)) \
{ \
*b->translated_eol = TRUE; \
\
/* Now that TRANSLATED_EOL has been set to TRUE, switch the \
translate_newline function that is used to the alternate which \
does not care about TRANSLATED_EOL */ \
b->translate_newline_fn = &translate_newline_alt; \
} \
} \
\
return SVN_NO_ERROR; \
}
DEFINE_TRANSLATE_NEWLINE_FN(translate_newline, 0)
DEFINE_TRANSLATE_NEWLINE_FN(translate_newline_alt, 1)
