Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed

[Daniel Trebbien]

On Fri, Oct 1, 2010 at 4:14 AM, Bert Huijben <b...@qqmail.nl> wrote:
>> > @@ -650,7 +651,13 @@ translate_newline(const char *eol_str,
>> >        *src_format_len = newline_len;
>> >      }
>> >    /* Translate the newline */
>> > -  return translate_write(dst, eol_str, eol_str_len);
>> > +  svn_error_t *err = translate_write(dst, eol_str, eol_str_len);
>>
>> No declarations mixed in with statements - we stick to C'89 rules.  But
>> I don't think there is any need to insert this new code *after* the
>> write - it can just as well go before the write, leaving the 'return'
>> how it was.
>
> The code can just use SVN_ERR() here, as you can't be sure the output is 
> available in error conditions anyway, so the extra check can be avoided on 
> errors.
>>
>> > +  if (eol_translated) {
>> > +    if (newline_len != eol_str_len ||
>> > +        strncmp(newline_buf, eol_str, newline_len))
>> > +      *eol_translated = TRUE;
>> > +  }
>> > +  return err;
>
> And this can be a return SVN_NO_ERROR;

I am not sure what the "extra check" is.

Is this preferred?:

  /* Translate the newline */
  SVN_ERR(translate_write(dst, eol_str, eol_str_len));

  if (translated_eol) {
    if (newline_len != eol_str_len ||
        strncmp(newline_buf, eol_str, newline_len))
      *translated_eol = TRUE;
  }
  return SVN_NO_ERROR;