Re: swig-py3 str/bytes difference in svn_config_*()

[Yasuhito FUTATSUKI]

Though this issue was already resolved...

On 2019/12/13 17:28, Yasuhito FUTATSUKI wrote:
> On 2019/12/13 16:24, Daniel Shahaf wrote:
>> Thanks for the quick fix.  However, it doesn't work on my machine (Debian 
>> stretch).
>>
>> With your latest patch, «svn_config_get_user_config_path(None, None)»
>> calls the C function of the name with «path == NULL» and segfaults 
>> immediately,
>> because the parameter is dereferenced on the first line of the function.  As 
>> to
>> the other case, I get a bogus stack trace:
>>
>> [[[
>> % PYTHONPATH=… lldb -- python3 -c $'from svn.core import *; 
>> svn_config_ensure("/tmp/foo")'
>> (lldb) r
>> Process 19706 launched: '/usr/bin/python3' (x86_64)
>> Process 19706 stopped
>> * thread #1, name = 'python3', stop reason = signal SIGSEGV: invalid address 
>> (fault address: 0x0)
>>  frame #0: 0x
>> error: memory read failed for 0x0
>> (lldb)
>> ]]]
> 
> Thank you for testing. I also tested and same result on FreeBSD
> with Python 3.6. It seems "z*" format in PyArg_ParseTuple() doesn't
> work I expected.

That was my misreading of the spec[1][2]. The argment type for "z*"
should be Py_buffer, not const char *. Similarly, "z#" can convert
str(unicode), bytes, None into "const char *" and int, but it is not
what we want here. Anyway we had to write additional code if we want to
use parse=... feature of SWIG typemap here.

https://docs.python.org/2.7/c-api/arg.html
https://docs.python.org/3/c-api/arg.html

Cheers,
-- 
Yasuhito FUATSUI  / 

"svn patch" and the TAB character

[Doug Robinson]

Folks:

If I use "svn diff --patch compatible" to produce a patch file.  Then edit
that patch file using an editor with settings that replace the TAB with a
"proper" number of SPACE characters, the "svn patch" command will declare:

$ svn patch ../n.txt
Skipped missing target: 'TheFile  (revision 18)'
Summary of conflicts:
  Skipped paths: 1

(the TAB was between "TheFile" and "(revision...)".  Changing the spaces
back to a tab enables the "svn patch" command to work.

I'm wondering if this is:
A. "goodness" as in "protection against diff chunks being
contaminated/modified" or
B. "bug" as in "the other patch tools out there don't enforce that TAB".

I looked through old postings and didn't see a discussion.

Thoughts?

Cheers.

Doug
-- 
*DOUGLAS B ROBINSON* SENIOR PRODUCT MANAGER

T +1 925 396 1125
*E* doug.robin...@wandisco.com

-- 


* *

**The *LiveData* Company
*Find out more 
*wandisco.com *



 



THIS MESSAGE AND ANY ATTACHMENTS 
ARE CONFIDENTIAL, PROPRIETARY AND MAY BE PRIVILEGED
*


If this message was 
misdirected, WANdisco, Inc. and its subsidiaries, ("WANdisco") does not 
waive any confidentiality or privilege. If you are not the intended 
recipient, please notify us immediately and destroy the message without 
disclosing its contents to anyone. Any distribution, use or copying of this 
email or the information it contains by other than an intended recipient is 
unauthorized. The views and opinions expressed in this email message are 
the author's own and may not reflect the views and opinions of WANdisco, 
unless the author is authorized by WANdisco to express such views or 
opinions on its behalf. All email sent to or from this address is subject 
to electronic storage and review by WANdisco. Although WANdisco operates 
anti-virus programs, it does not accept responsibility for any damage 
whatsoever caused by viruses being passed.

Re: swig-py3 str/bytes difference in svn_config_*()

[Yasuhito FUTATSUKI]

On 2019/12/13 20:44, Yasuhito FUTATSUKI wrote:
> On 2019/12/13 19:47, Daniel Shahaf wrote:
>> Yasuhito FUTATSUKI wrote on Fri, Dec 13, 2019 at 17:28:18 +0900:
>>> Thank you for testing. I also tested and same result on FreeBSD
>>> with Python 3.6. It seems "z*" format in PyArg_ParseTuple() doesn't
>>> work I expected.
>>>
>>> On the other hand, I can't find good reason to care typemap for
>>> "const char *config_dir" input different from other MAY_BE_NULL types
>>> on Python bindings only, so I removed this special handling.
>>> Then, it works :)
>>
>> I don't see any reason either, and I confirm this version of the patch fixes
>> both issues.  Thanks again for the quick fix.
> 
> It seems this extra typemap have been forgotten to remove when
> MAY_BE_NULL was introduced.
> 
> I'll commit this patch after testing on Python 2.7.

Done in r1871347 and r1871349.

Cheers,
-- 
Yasuhito FUTATSUKI  / 

Re: swig-py3 str/bytes difference in svn_config_*()

[Yasuhito FUTATSUKI]

On 2019/12/13 19:47, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Fri, Dec 13, 2019 at 17:28:18 +0900:
>> Thank you for testing. I also tested and same result on FreeBSD
>> with Python 3.6. It seems "z*" format in PyArg_ParseTuple() doesn't
>> work I expected.
>>
>> On the other hand, I can't find good reason to care typemap for
>> "const char *config_dir" input different from other MAY_BE_NULL types
>> on Python bindings only, so I removed this special handling.
>> Then, it works :)
> 
> I don't see any reason either, and I confirm this version of the patch fixes
> both issues.  Thanks again for the quick fix.

It seems this extra typemap have been forgotten to remove when
MAY_BE_NULL was introduced.

I'll commit this patch after testing on Python 2.7.

> I forgot to say this earlier, but I ran into some problems trying to apply 
> your
> patch.  The indentation was corrupted: the line that start with spaces, above,
> had an extra leading space.  To cut a long story short, your email was 
> formatted
> with format=flowed, which means two things:
> 
> - Your MUA correctly space-stuffed the unidiff context lines, but on my end,
>   Mutt did not un-space-stuff them before piping them to patch(1).  Therefore,
>   patch(1) saw one leading space too many on the context lines, and 
> (correctly)
>   failed.
> 
> - In general, format=flowed and inline patches don't mix — IIRC, the problem 
> is
>   that unidiffs that contain lines with trailing spaces will be corrupted — so
>   you should either disable format=flowed on your end or send patches as
>   attachments (as opposed to inline).  See:
>   https://wiki.openstack.org/wiki/MailingListEtiquette#Thunderbird

Thank you for this informaiton. I've changed my Thunderbird setting
(at least this one, and I'll do it on other environments).

Cheers,
-- 
Yasuhito FUTATSUKI 

Re: [#4840] Merge assertion failure in svn_sort__array_insert

[Julian Foad]

Here are the three patches resulting from my work so far:

merge-4840-backtrace-4.patch
merge-4840-print-empty-range.patch
merge-4840-test-1.patch

Together, anyone who wants to follow along can reproduce the errors. 
These are not quite in a clean enough state to commit, but I expect to 
commit something like them.


- Julian
For issue #4840, "Merge assertion failure in svn_sort__array_insert".


* subversion/libsvn_subr/mergeinfo.c
  (range_to_string): Print an empty range in the form "[empty-range@100*]".
--This line, and those below, will be ignored--

Index: subversion/libsvn_subr/mergeinfo.c
===
--- subversion/libsvn_subr/mergeinfo.c	(revision 1871324)
+++ subversion/libsvn_subr/mergeinfo.c	(working copy)
@@ -470,6 +470,8 @@ range_to_string(const svn_merge_range_t
 return apr_psprintf(pool, "-%ld%s", range->start, mark);
   else if (range->start < range->end)
 return apr_psprintf(pool, "%ld-%ld%s", range->start + 1, range->end, mark);
+  else if (range->start == range->end)
+return apr_psprintf(pool, "[empty-range@%ld%s]", range->start, mark);
   else
 return apr_psprintf(pool, "%ld-%ld%s", range->start, range->end + 1, mark);
 }
Random-input testing for issue #4840, "Merge assertion failure in
svn_sort__array_insert".

### Manually edit the test to call one of
rangelist_random_non_canonical() or
rangelist_random_semi_canonical() or
rangelist_random_canonical(), in two places.

Note that the number of iterations needs to be in the order of 1000 in each
loop rather than 300 in each loop, in its present formulation, to catch some
of the errors.

* subversion/tests/libsvn_subr/mergeinfo-test.c
  (...): Helper functions.
  (test_rangelist_merge_random_inputs): New test.
  (test_funcs): Run it.
--This line, and those below, will be ignored--

Index: subversion/tests/libsvn_subr/mergeinfo-test.c
===
--- subversion/tests/libsvn_subr/mergeinfo-test.c	(revision 1871324)
+++ subversion/tests/libsvn_subr/mergeinfo-test.c	(working copy)
@@ -30,12 +30,13 @@
 
 #include "svn_hash.h"
 #include "svn_pools.h"
 #include "svn_types.h"
 #include "svn_mergeinfo.h"
 #include "private/svn_mergeinfo_private.h"
+#include "private/svn_sorts_private.h"
 #include "../svn_test.h"
 
 /* A quick way to create error messages.  */
 static svn_error_t *
 fail(apr_pool_t *pool, const char *fmt, ...)
 {
@@ -1780,12 +1781,258 @@ test_rangelist_loop(apr_pool_t *pool)
   /* TODO: Verify result */
 }
   }
 
   return SVN_NO_ERROR;
 }
+
+/* A representation of svn_rangelist_t in which
+ *   root[R]:= (revision R is in the rangelist)
+ *   inherit[R] := (revision R is in the rangelist and inheritable)
+ *
+ * Assuming all forward ranges.
+ */
+typedef struct rl_array_t {
+svn_boolean_t root[100];
+svn_boolean_t inherit[100];
+} rl_array_t;
+
+static void
+rangelist_to_array(rl_array_t *a,
+   const svn_rangelist_t *rl)
+{
+  int i;
+
+  memset(a, 0, sizeof(*a));
+  for (i = 0; i < rl->nelts; i++)
+{
+  svn_merge_range_t *range = APR_ARRAY_IDX(rl, i, svn_merge_range_t *);
+  int r;
+
+  for (r = range->start + 1; r <= range->end; r++)
+{
+  a->root[r] = TRUE;
+  a->inherit[r] = range->inheritable;
+}
+}
+}
+
+/* Let MA := union(BA, CA)
+ * Assuming all forward ranges.
+ */
+static void
+rangelist_array_merge(rl_array_t *ma,
+  const rl_array_t *ba,
+  const rl_array_t *ca)
+{
+  int r;
+
+  for (r = 0; r < 100; r++)
+{
+  ma->root[r]= ba->root[r]|| ca->root[r];
+  ma->inherit[r] = ba->inherit[r] || ca->inherit[r];
+}
+}
+
+static svn_boolean_t
+rangelist_array_equal(const rl_array_t *ba,
+  const rl_array_t *ca)
+{
+  int r;
+
+  for (r = 0; r < 100; r++)
+{
+  if (ba->root[r]!= ca->root[r]
+   || ba->inherit[r] != ca->inherit[r])
+{
+  return FALSE;
+}
+}
+  return TRUE;
+}
+
+static svn_boolean_t
+rangelist_equal(const svn_rangelist_t *bl,
+const svn_rangelist_t *cl)
+{
+  rl_array_t ba, ca;
+
+  rangelist_to_array(, bl);
+  rangelist_to_array(, cl);
+  return rangelist_array_equal(, );
+}
+
+#define IS_VALID_FORWARD_RANGE(range) \
+  (SVN_IS_VALID_REVNUM((range)->start) && ((range)->start < (range)->end))
+
+/* Check rangelist is sorted and contains forward ranges. */
+static svn_boolean_t
+rangelist_is_sorted(const svn_rangelist_t *rangelist)
+{
+  int i;
+
+  for (i = 0; i < rangelist->nelts; i++)
+{
+  const svn_merge_range_t *thisrange
+= APR_ARRAY_IDX(rangelist, i, svn_merge_range_t *);
+
+  if (!IS_VALID_FORWARD_RANGE(thisrange))
+return FALSE;
+}
+  for (i = 1; i < rangelist->nelts; i++)
+{
+  const svn_merge_range_t *lastrange
+= APR_ARRAY_IDX(rangelist, i-1, svn_merge_range_t *);
+  

Re: swig-py3 str/bytes difference in svn_config_*()

[Daniel Shahaf]

Yasuhito FUTATSUKI wrote on Fri, Dec 13, 2019 at 17:28:18 +0900:
> Thank you for testing. I also tested and same result on FreeBSD
> with Python 3.6. It seems "z*" format in PyArg_ParseTuple() doesn't
> work I expected.
> 
> On the other hand, I can't find good reason to care typemap for
> "const char *config_dir" input different from other MAY_BE_NULL types
> on Python bindings only, so I removed this special handling.
> Then, it works :)

I don't see any reason either, and I confirm this version of the patch fixes
both issues.  Thanks again for the quick fix.

> [[[
> Index: subversion/bindings/swig/core.i
> ===
> --- subversion/bindings/swig/core.i (revision 1871319)
> +++ subversion/bindings/swig/core.i (working copy)
> @@ -362,7 +362,9 @@
>  /* svn_config_get */
>  const char *default_value,
>  /* svn_config_read_auth_data */
> -const char *config_dir,
> +const char *config_dir,

I forgot to say this earlier, but I ran into some problems trying to apply your
patch.  The indentation was corrupted: the line that start with spaces, above,
had an extra leading space.  To cut a long story short, your email was formatted
with format=flowed, which means two things:

- Your MUA correctly space-stuffed the unidiff context lines, but on my end,
  Mutt did not un-space-stuff them before piping them to patch(1).  Therefore,
  patch(1) saw one leading space too many on the context lines, and (correctly)
  failed.

- In general, format=flowed and inline patches don't mix — IIRC, the problem is
  that unidiffs that contain lines with trailing spaces will be corrupted — so
  you should either disable format=flowed on your end or send patches as
  attachments (as opposed to inline).  See:
  https://wiki.openstack.org/wiki/MailingListEtiquette#Thunderbird

Cheers,

Daniel

Re: swig-py3 str/bytes difference in svn_config_*()

[Yasuhito FUTATSUKI]

On 2019/12/13 16:24, Daniel Shahaf wrote:

Thanks for the quick fix.  However, it doesn't work on my machine (Debian 
stretch).

With your latest patch, «svn_config_get_user_config_path(None, None)»
calls the C function of the name with «path == NULL» and segfaults immediately,
because the parameter is dereferenced on the first line of the function.  As to
the other case, I get a bogus stack trace:

[[[
% PYTHONPATH=… lldb -- python3 -c $'from svn.core import *; 
svn_config_ensure("/tmp/foo")'
(lldb) r
Process 19706 launched: '/usr/bin/python3' (x86_64)
Process 19706 stopped
* thread #1, name = 'python3', stop reason = signal SIGSEGV: invalid address 
(fault address: 0x0)
 frame #0: 0x
error: memory read failed for 0x0
(lldb)
]]]


Thank you for testing. I also tested and same result on FreeBSD
with Python 3.6. It seems "z*" format in PyArg_ParseTuple() doesn't
work I expected.

On the other hand, I can't find good reason to care typemap for
"const char *config_dir" input different from other MAY_BE_NULL types
on Python bindings only, so I removed this special handling.
Then, it works :)

[[[
Index: subversion/bindings/swig/core.i
===
--- subversion/bindings/swig/core.i (revision 1871319)
+++ subversion/bindings/swig/core.i (working copy)
@@ -362,7 +362,9 @@
 /* svn_config_get */
 const char *default_value,
 /* svn_config_read_auth_data */
-const char *config_dir,
+const char *config_dir,
+/* svn_config_get_user_config_path */
+const char *fname,
 /* svn_diff_file_output_merge */
 const char *conflict_original,
 const char *conflict_modified,
@@ -723,11 +725,6 @@
   svn_swig_rb_config_section_enumerator)
 #endif
 
-/* Allow None to be passed as config_dir argument */

-#ifdef SWIGPYTHON
-%typemap(in,parse="z") const char *config_dir "";
-#endif
-
 /* ---
   thunk the various authentication prompt functions.
   PERL NOTE: store the inputed SV in _global_callback for use in the
]]]

Thanks,
--
Yasuhito FUTATSUKI  /