[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] binary property values in Perl API

From: David James <djames_at_collab.net>
Date: 2006-09-14 08:04:40 CEST

On 9/13/06, Geoff Richards <qef@ungwe.org> wrote:
> On Wed, Sep 13, 2006 at 05:37:33AM +0100, Geoff Richards wrote:
> > Warning: this patch is untested. The modified file does compile without
> > any additional warnings, but I can't currently compile or test the Perl
> > bindings as a whole. Someone who knows how this stuff works needs to
> > give it a spin, but I think it should work.
> >
> > This should fix the passing of binary values for properties between C
> > and Perl. Currently Perl delta editors receive truncated values for
> > properties which contain null bytes.
>
> OK, I've tested the patch and it seems to work fine. I've added a test
> which fails without it, but passes with the fix added. This also
> includes some other new testing code for SVN::Fs and SVN::Ra which I
> was working on anyway. These two test files all pass as of r21492.
>
> So, new version of the patch attached. The C code changes are the same
> as last time.

Great patch, Geoff! I almost committed your first patch, but I wanted
to see some tests first.

Thanks a lot for submitting these tests as they will be very helpful
in the long run. In combination with the extra documentation you
submitted yesterday, this should be a big boost for the Perl bindings.

I have one request for you. In future, could you include a log message
with your patches? This log message should include the names of the
functions which you modify, and a description of what you changed in
each function. I've included an example log message (for this patch)
below to help explain.

[[[

Upgrade the SWIG/perl bindings to support binary properties, including
null bytes. In swigutil_pl.c, we no longer treat "svn_string_t" objects
as null-terminated strings; instead, we look at the length parameter of
the string to determine its length.

[ In subversion/bindings/swig/perl ]

* libsvn_swig_perl/swigutil_pl.c
  (svn_swig_pl_callback_thunk): Add 't' format code for svn_string_t
  datatype.
  (thunk_change_dir_prop, thunk_change_file_prop): Use 't' format code
  for svn_string_t objects.
  (thunk_get_wc_prop): Don't treat the return value of get_wc_prop as a
  null-terminated string when we convert it into an svn_string_t;
  instead, look at its length field.

* native/t/6ra.t, native/t/2fs.t:
  Add a bunch of tests for binary properties. Add additional checks
  throughout. Fix a few spots where the indentation was off. Some of
  the new tests are skipped on Windows because they are dependent
  on a pre-revprop-change hook. The pre-revprop-change hook is a bit
  trickier to setup on Windows and Mac OS X, so I've punted on that for
  now by skipping the tests.

]]]

I committed your patch with the above log message in r21494. Let me
know if I missed anything.

Thanks for contributing to Subversion!

David

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Sep 14 08:04:59 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.