Author: stsp
Date: Thu Jul  8 23:48:58 2010
New Revision: 962356

URL: http://svn.apache.org/viewvc?rev=962356&view=rev
Log:
Make svn_hunk_t an opaque type.

This makes sure that nothing outside the diff parser has access to the
raw hunk text streams. These streams should only be read via the public
svn_diff_hunk_readline_* accessor functions, and not via the generic
stream read or readline functions.

* subversion/include/svn_diff.h
  (svn_hunk_t): Just typedef this here, instead of declaring it.
   Parts of docstrings remain, in particular the explanation of
   what the various hunk texts are.
  (svn_diff_hunk_reset_diff_text,
   svn_diff_hunk_reset_original_text,
   svn_diff_hunk_reset_modified_text,
   svn_diff_hunk_get_original_start,
   svn_diff_hunk_get_original_length,
   svn_diff_hunk_get_modified_start,
   svn_diff_hunk_get_modified_length,
   svn_diff_hunk_get_leading_context,
   svn_diff_hunk_get_trailing_context): Declare.
  (svn_diff_hunk_readline_diff_text,
   svn_diff_hunk_readline_original_text,
   svn_diff_hunk_readline_modified_text): Move these declarations up a bit
    so that all svn_diff_hunk_* declarations are grouped together.

* subversion/libsvn_diff/parse-diff.c
  (svn_hunk_t): Declare here.
  (svn_diff_hunk_reset_diff_text,
   svn_diff_hunk_reset_original_text,
   svn_diff_hunk_reset_modified_text,
   svn_diff_hunk_get_original_start,
   svn_diff_hunk_get_original_length,
   svn_diff_hunk_get_modified_start,
   svn_diff_hunk_get_modified_length,
   svn_diff_hunk_get_leading_context,
   svn_diff_hunk_get_trailing_context): New accessor functions for
    fields in svn_hunk_t.

* subversion/libsvn_client/patch.c
  (match_hunk, get_hunk_info, reject_hunk, apply_hunk,
   send_patch_notification): Use svn_diff_hunk_* accessor functions
    instead of looking at fields in svn_hunk_t directly.

Modified:
    subversion/trunk/subversion/include/svn_diff.h
    subversion/trunk/subversion/libsvn_client/patch.c
    subversion/trunk/subversion/libsvn_diff/parse-diff.c

Modified: subversion/trunk/subversion/include/svn_diff.h
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_diff.h?rev=962356&r1=962355&r2=962356&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_diff.h (original)
+++ subversion/trunk/subversion/include/svn_diff.h Thu Jul  8 23:48:58 2010
@@ -786,78 +786,151 @@ typedef enum svn_diff_operation_kind_e
 }svn_diff_operation_kind_t;
 
 /**
- * A single hunk inside a patch
+ * A single hunk inside a patch.
+ *
+ * The lines of text comprising the hunk can be interpreted in three ways:
+ *   - diff text       The hunk as it appears in the unidiff patch file,
+ *                     including the hunk header line ("@@ ... @@")
+ *   - original text   The text the patch was based on.
+ *   - modified text   The result of patching the original text.
+ *
+ * For example, consider a hunk with the following diff text:
+ *   @@ -1,5 +1,5 @@
+ *    #include <stdio.h>
+ *    int main(int argc, char *argv[]) {
+ *   -        printf("Hello World!\n");
+ *   +        printf("I like Subversion!\n");
+ *    }
+ *
+ * The original text of this hunk is:
+ *   #include <stdio.h>
+ *   int main(int argc, char *argv[]) {
+ *           printf("Hello World!\n");
+ *   }
+ *
+ * And the modified text is:
+ *   #include <stdio.h>
+ *   int main(int argc, char *argv[]) {
+ *           printf("I like Subversion!\n");
+ *   }
+ *
+ * @see svn_diff_hunk_readline_diff_text()
+ * @see svn_diff_hunk_readline_original_text()
+ * @see svn_diff_hunk_readline_modified_text()
  *
  * @since New in 1.7. */
-typedef struct svn_hunk_t {
-  /**
-   * The hunk's unidiff text as it appeared in the patch file,
-   * without range information.
-   * This stream should not be read from directly, because it will
-   * return the wrong result for reversed hunks.
-   * @see svn_diff_hunk_readline_diff_text()
-   * However, the stream supports resetting and it is safe to reset it.
-   */
-  svn_stream_t *diff_text;
+typedef struct svn_hunk_t svn_hunk_t;
 
-  /**
-   * Whether the hunk is being interpreted in reverse.
-   */
-  svn_boolean_t reverse;
+/**
+ * Allocate @a *stringbuf in @a result_pool, and read into it one line
+ * of the diff text of @a hunk. The first line returned is the hunk header.
+ * Any subsequent lines are unidiff data (starting with '+', '-', or ' ').
+ * If the @a hunk is being interpreted in reverse (i.e. the reverse
+ * parameter of svn_diff_parse_next_patch() was @c TRUE), the diff
+ * text will be returned in reversed form.
+ * The line-terminator is detected automatically and stored in @a *eol
+ * if @a eol is not NULL.
+ * If EOF is reached and the stream does not end with a newline character,
+ * and @a eol is not NULL, @a *eol is set to NULL.
+ * Temporary allocations will be performed in @a scratch_pool.
+ *
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_diff_hunk_readline_diff_text(const svn_hunk_t *hunk,
+                                 svn_stringbuf_t **stringbuf,
+                                 const char **eol,
+                                 svn_boolean_t *eof,
+                                 apr_pool_t *result_pool,
+                                 apr_pool_t *scratch_pool);
 
-  /**
-   * The original and modified texts in the hunk range.
-   * Derived from the diff text.
-   *
-   * For example, consider a hunk such as:
-   *   @@ -1,5 +1,5 @@
-   *    #include <stdio.h>
-   *    int main(int argc, char *argv[])
-   *    {
-   *   -        printf("Hello World!\n");
-   *   +        printf("I like Subversion!\n");
-   *    }
-   *
-   * Then, the original text described by the hunk is:
-   *   #include <stdio.h>
-   *   int main(int argc, char *argv[])
-   *   {
-   *           printf("Hello World!\n");
-   *   }
-   *
-   * And the modified text described by the hunk is:
-   *   #include <stdio.h>
-   *   int main(int argc, char *argv[])
-   *   {
-   *           printf("I like Subversion!\n");
-   *   }
-   *
-   * These streams should not be read from directly.
-   * Reading them with svn_stream_read() or svn_stream_readline() will
-   * not yield the expected result, because that will return the unidiff
-   * text from the patch file unmodified.
-   * @see svn_diff_hunk_readline_original_text()
-   * @see svn_diff_hunk_readline_modified_text()
-   *
-   * However, the streams support resetting and it is safe to reset them.
-   */
-  svn_stream_t *original_text;
-  svn_stream_t *modified_text;
+/**
+ * Allocate @a *stringbuf in @a result_pool, and read into it one line
+ * of the original text of @a hunk.
+ * The line-terminator is detected automatically and stored in @a *eol
+ * if @a eol is not NULL.
+ * If EOF is reached and the stream does not end with a newline character,
+ * and @a eol is not NULL, @a *eol is set to NULL.
+ * Temporary allocations will be performed in @a scratch_pool.
+ *
+ * @see svn_hunk_t
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_diff_hunk_readline_original_text(const svn_hunk_t *hunk,
+                                     svn_stringbuf_t **stringbuf,
+                                     const char **eol,
+                                     svn_boolean_t *eof,
+                                     apr_pool_t *result_pool,
+                                     apr_pool_t *scratch_pool);
 
-  /**
-   * Hunk ranges as they appeared in the patch file.
-   * All numbers are lines, not bytes. */
-  svn_linenum_t original_start;
-  svn_linenum_t original_length;
-  svn_linenum_t modified_start;
-  svn_linenum_t modified_length;
-
-  /** Number of lines starting with ' ' before first '+' or '-'. */
-  svn_linenum_t leading_context;
-
-  /** Number of lines starting with ' ' after last '+' or '-'. */
-  svn_linenum_t trailing_context;
-} svn_hunk_t;
+/**
+ * Like svn_diff_hunk_readline_original_text(), but it returns lines from
+ * the modified text of the hunk.
+ *
+ * @see svn_hunk_t
+ * @since New in 1.7.
+ */
+svn_error_t *
+svn_diff_hunk_readline_modified_text(const svn_hunk_t *hunk,
+                                     svn_stringbuf_t **stringbuf,
+                                     const char **eol,
+                                     svn_boolean_t *eof,
+                                     apr_pool_t *result_pool,
+                                     apr_pool_t *scratch_pool);
+
+/* Reset the diff text of @a hunk so it can be read again from the start.
+ * @since New in 1.7. */
+svn_error_t *
+svn_diff_hunk_reset_diff_text(const svn_hunk_t *hunk);
+
+/* Reset the original text of @a hunk so it can be read again from the start.
+ * @since New in 1.7. */
+svn_error_t *
+svn_diff_hunk_reset_original_text(const svn_hunk_t *hunk);
+
+/* Reset the modified text of @a hunk so it can be read again from the start.
+ * @since New in 1.7. */
+svn_error_t *
+svn_diff_hunk_reset_modified_text(const svn_hunk_t *hunk);
+
+/* Return the line offset of the original hunk text,
+ * as parsed from the hunk header.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_original_start(const svn_hunk_t *hunk);
+
+/* Return the number of lines in the original @a hunk text,
+ * as parsed from the hunk header.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_original_length(const svn_hunk_t *hunk);
+
+/* Return the line offset of the modified @a hunk text,
+ * as parsed from the hunk header.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_modified_start(const svn_hunk_t *hunk);
+
+/* Return the number of lines in the modified @a hunk text,
+ * as parsed from the hunk header.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_modified_length(const svn_hunk_t *hunk);
+
+/* Return the number of lines of leading context of @a hunk,
+ * i.e. the number of lines starting with ' ' before the first line
+ * that starts with a '+' or '-'.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_leading_context(const svn_hunk_t *hunk);
+
+/* Return the number of lines of trailing context of @a hunk,
+ * i.e. the number of lines starting with ' ' after the last line
+ * that starts with a '+' or '-'.
+ * @since New in 1.7. */
+svn_linenum_t
+svn_diff_hunk_get_trailing_context(const svn_hunk_t *hunk);
 
 /**
  * Data type to manage parsing of patches.
@@ -911,64 +984,6 @@ svn_diff_parse_next_patch(svn_patch_t **
                           apr_pool_t *scratch_pool);
 
 /**
- * Allocate @a *stringbuf in @a result_pool, and read into it one line
- * of the original text of @a hunk.
- * The line-terminator is detected automatically and stored in @a *eol
- * if @a eol is not NULL.
- * If EOF is reached and the stream does not end with a newline character,
- * and @a eol is not NULL, @a *eol is set to NULL.
- * Temporary allocations will be performed in @a scratch_pool.
- *
- * @see svn_hunk_t
- * @since New in 1.7.
- */
-svn_error_t *
-svn_diff_hunk_readline_original_text(const svn_hunk_t *hunk,
-                                     svn_stringbuf_t **stringbuf,
-                                     const char **eol,
-                                     svn_boolean_t *eof,
-                                     apr_pool_t *result_pool,
-                                     apr_pool_t *scratch_pool);
-
-/**
- * Like svn_diff_hunk_readline_original_text(), but it returns lines from
- * the modified text of the hunk.
- *
- * @see svn_hunk_t
- * @since New in 1.7.
- */
-svn_error_t *
-svn_diff_hunk_readline_modified_text(const svn_hunk_t *hunk,
-                                     svn_stringbuf_t **stringbuf,
-                                     const char **eol,
-                                     svn_boolean_t *eof,
-                                     apr_pool_t *result_pool,
-                                     apr_pool_t *scratch_pool);
-
-/**
- * Allocate @a *stringbuf in @a result_pool, and read into it one line
- * of the diff text of @a hunk. The first line returned is the hunk header.
- * Any subsequent lines are unidiff data (starting with '+', '-', or ' ').
- * If the @a hunk is being interpreted in reverse (i.e. the reverse
- * parameter of svn_diff_parse_next_patch() was @c TRUE), the diff
- * text will be returned in reversed form.
- * The line-terminator is detected automatically and stored in @a *eol
- * if @a eol is not NULL.
- * If EOF is reached and the stream does not end with a newline character,
- * and @a eol is not NULL, @a *eol is set to NULL.
- * Temporary allocations will be performed in @a scratch_pool.
- *
- * @since New in 1.7.
- */
-svn_error_t *
-svn_diff_hunk_readline_diff_text(const svn_hunk_t *hunk,
-                                 svn_stringbuf_t **stringbuf,
-                                 const char **eol,
-                                 svn_boolean_t *eof,
-                                 apr_pool_t *result_pool,
-                                 apr_pool_t *scratch_pool);
-
-/**
  * Dispose of @a patch, closing any streams used by it.
  *
  * @since New in 1.7.

Modified: subversion/trunk/subversion/libsvn_client/patch.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/patch.c?rev=962356&r1=962355&r2=962356&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/patch.c (original)
+++ subversion/trunk/subversion/libsvn_client/patch.c Thu Jul  8 23:48:58 2010
@@ -613,6 +613,9 @@ match_hunk(svn_boolean_t *matched, patch
   svn_boolean_t hunk_eof;
   svn_boolean_t lines_matched;
   apr_pool_t *iterpool;
+  svn_linenum_t original_length;
+  svn_linenum_t leading_context;
+  svn_linenum_t trailing_context;
 
   *matched = FALSE;
 
@@ -622,7 +625,10 @@ match_hunk(svn_boolean_t *matched, patch
   saved_line = target->current_line;
   lines_read = 0;
   lines_matched = FALSE;
-  SVN_ERR(svn_stream_reset(hunk->original_text));
+  original_length = svn_diff_hunk_get_original_length(hunk);
+  leading_context = svn_diff_hunk_get_leading_context(hunk);
+  trailing_context = svn_diff_hunk_get_trailing_context(hunk);
+  SVN_ERR(svn_diff_hunk_reset_original_text(hunk));
   iterpool = svn_pool_create(pool);
   do
     {
@@ -644,10 +650,10 @@ match_hunk(svn_boolean_t *matched, patch
       SVN_ERR(read_line(target, &target_line, iterpool, iterpool));
       if (! hunk_eof)
         {
-          if (lines_read <= fuzz && hunk->leading_context > fuzz)
+          if (lines_read <= fuzz && leading_context > fuzz)
             lines_matched = TRUE;
-          else if (lines_read > hunk->original_length - fuzz &&
-                   hunk->trailing_context > fuzz)
+          else if (lines_read > original_length - fuzz &&
+                   trailing_context > fuzz)
             lines_matched = TRUE;
           else
             {
@@ -740,8 +746,8 @@ scan_for_match(svn_linenum_t *matched_li
               hi = APR_ARRAY_IDX(target->hunks, i, const hunk_info_t *);
               taken = (! hi->rejected &&
                        target->current_line >= hi->matched_line &&
-                       target->current_line < hi->matched_line +
-                                              hi->hunk->original_length);
+                       target->current_line < (hi->matched_line +
+                         svn_diff_hunk_get_original_length(hi->hunk)));
               if (taken)
                 break;
             }
@@ -779,38 +785,41 @@ get_hunk_info(hunk_info_t **hi, patch_ta
               apr_pool_t *result_pool, apr_pool_t *scratch_pool)
 {
   svn_linenum_t matched_line;
+  svn_linenum_t original_start;
+
+  original_start = svn_diff_hunk_get_original_start(hunk);
 
   /* An original offset of zero means that this hunk wants to create
    * a new file. Don't bother matching hunks in that case, since
    * the hunk applies at line 1. If the file already exists, the hunk
    * is rejected. */
-  if (hunk->original_start == 0)
+  if (original_start == 0)
     {
       if (target->kind_on_disk == svn_node_file)
         matched_line = 0;
       else
         matched_line = 1;
     }
-  else if (hunk->original_start > 0 && target->kind_on_disk == svn_node_file)
+  else if (original_start > 0 && target->kind_on_disk == svn_node_file)
     {
       svn_linenum_t saved_line = target->current_line;
 
       /* Scan for a match at the line where the hunk thinks it
        * should be going. */
-      SVN_ERR(seek_to_line(target, hunk->original_start, scratch_pool));
-      if (target->current_line != hunk->original_start)
+      SVN_ERR(seek_to_line(target, original_start, scratch_pool));
+      if (target->current_line != original_start)
         {
           /* Seek failed. */
           matched_line = 0;
         }
       else
         SVN_ERR(scan_for_match(&matched_line, target, hunk, TRUE,
-                               hunk->original_start + 1, fuzz,
+                               original_start + 1, fuzz,
                                ignore_whitespace,
                                cancel_func, cancel_baton,
                                scratch_pool));
 
-      if (matched_line != hunk->original_start)
+      if (matched_line != original_start)
         {
           /* Scan the whole file again from the start. */
           SVN_ERR(seek_to_line(target, 1, scratch_pool));
@@ -818,7 +827,7 @@ get_hunk_info(hunk_info_t **hi, patch_ta
           /* Scan forward towards the hunk's line and look for a line
            * where the hunk matches. */
           SVN_ERR(scan_for_match(&matched_line, target, hunk, FALSE,
-                                 hunk->original_start, fuzz,
+                                 original_start, fuzz,
                                  ignore_whitespace,
                                  cancel_func, cancel_baton,
                                  scratch_pool));
@@ -913,10 +922,10 @@ reject_hunk(patch_target_t *target, hunk
   apr_pool_t *iterpool;
 
   hunk_header = apr_psprintf(pool, "@@ -%lu,%lu +%lu,%lu @@%s",
-                             hi->hunk->original_start,
-                             hi->hunk->original_length,
-                             hi->hunk->modified_start,
-                             hi->hunk->modified_length,
+                             svn_diff_hunk_get_original_start(hi->hunk),
+                             svn_diff_hunk_get_original_length(hi->hunk),
+                             svn_diff_hunk_get_modified_start(hi->hunk),
+                             svn_diff_hunk_get_modified_length(hi->hunk),
                              APR_EOL_STR);
   len = strlen(hunk_header);
   SVN_ERR(svn_stream_write(target->reject, hunk_header, &len));
@@ -972,7 +981,8 @@ apply_hunk(patch_target_t *target, hunk_
 
       /* Skip the target's version of the hunk.
        * Don't skip trailing lines which matched with fuzz. */
-      line = target->current_line + hi->hunk->original_length - (2 * hi->fuzz);
+      line = target->current_line +
+             svn_diff_hunk_get_original_length(hi->hunk) - (2 * hi->fuzz);
       SVN_ERR(seek_to_line(target, line, pool));
       if (target->current_line != line && ! target->eof)
         {
@@ -999,7 +1009,7 @@ apply_hunk(patch_target_t *target, hunk_
                                                    iterpool, iterpool));
       lines_read++;
       if (! eof && lines_read > hi->fuzz &&
-          lines_read <= hi->hunk->modified_length - hi->fuzz)
+          lines_read <= svn_diff_hunk_get_modified_length(hi->hunk) - hi->fuzz)
         {
           if (hunk_line->len >= 1)
             SVN_ERR(try_stream_write(target->patched, target->patched_path,
@@ -1095,10 +1105,14 @@ send_patch_notification(const patch_targ
                                             ? target->local_abspath
                                             : target->local_relpath,
                                         action, pool);
-          notify->hunk_original_start = hi->hunk->original_start;
-          notify->hunk_original_length = hi->hunk->original_length;
-          notify->hunk_modified_start = hi->hunk->modified_start;
-          notify->hunk_modified_length = hi->hunk->modified_length;
+          notify->hunk_original_start =
+            svn_diff_hunk_get_original_start(hi->hunk);
+          notify->hunk_original_length =
+            svn_diff_hunk_get_original_length(hi->hunk);
+          notify->hunk_modified_start =
+            svn_diff_hunk_get_modified_start(hi->hunk);
+          notify->hunk_modified_length =
+            svn_diff_hunk_get_modified_length(hi->hunk);
           notify->hunk_matched_line = hi->matched_line;
           notify->hunk_fuzz = hi->fuzz;
 

Modified: subversion/trunk/subversion/libsvn_diff/parse-diff.c
URL: 
http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_diff/parse-diff.c?rev=962356&r1=962355&r2=962356&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_diff/parse-diff.c (original)
+++ subversion/trunk/subversion/libsvn_diff/parse-diff.c Thu Jul  8 23:48:58 
2010
@@ -39,6 +39,81 @@
 #define starts_with(str, start)  \
   (strncmp((str), (start), strlen(start)) == 0)
 
+struct svn_hunk_t {
+  /* Hunk texts (see include/svn_diff.h). */
+  svn_stream_t *diff_text;
+  svn_stream_t *original_text;
+  svn_stream_t *modified_text;
+
+  /* Whether the hunk is being interpreted in reverse. */
+  svn_boolean_t reverse;
+
+  /* Hunk ranges as they appeared in the patch file.
+   * All numbers are lines, not bytes. */
+  svn_linenum_t original_start;
+  svn_linenum_t original_length;
+  svn_linenum_t modified_start;
+  svn_linenum_t modified_length;
+
+  /* Number of lines of leading and trailing hunk context. */
+  svn_linenum_t leading_context;
+  svn_linenum_t trailing_context;
+};
+
+svn_error_t *
+svn_diff_hunk_reset_diff_text(const svn_hunk_t *hunk)
+{
+  return svn_error_return(svn_stream_reset(hunk->diff_text));
+}
+
+svn_error_t *
+svn_diff_hunk_reset_original_text(const svn_hunk_t *hunk)
+{
+  return svn_error_return(svn_stream_reset(hunk->original_text));
+}
+
+svn_error_t *
+svn_diff_hunk_reset_modified_text(const svn_hunk_t *hunk)
+{
+  return svn_error_return(svn_stream_reset(hunk->modified_text));
+}
+
+svn_linenum_t
+svn_diff_hunk_get_original_start(const svn_hunk_t *hunk)
+{
+  return hunk->original_start;
+}
+
+svn_linenum_t
+svn_diff_hunk_get_original_length(const svn_hunk_t *hunk)
+{
+  return hunk->original_length;
+}
+
+svn_linenum_t
+svn_diff_hunk_get_modified_start(const svn_hunk_t *hunk)
+{
+  return hunk->modified_start;
+}
+
+svn_linenum_t
+svn_diff_hunk_get_modified_length(const svn_hunk_t *hunk)
+{
+  return hunk->modified_length;
+}
+
+svn_linenum_t
+svn_diff_hunk_get_leading_context(const svn_hunk_t *hunk)
+{
+  return hunk->leading_context;
+}
+
+svn_linenum_t
+svn_diff_hunk_get_trailing_context(const svn_hunk_t *hunk)
+{
+  return hunk->trailing_context;
+}
+
 /* Try to parse a positive number from a decimal number encoded
  * in the string NUMBER. Return parsed number in OFFSET, and return
  * TRUE if parsing was successful. */


Reply via email to