Re: [PATCH] svn info - changing hard coded error message to specific one.
Julian Foad writes:
> On Fri, 2010-12-24, C. Michael Pilato wrote:
>
>> On 12/23/2010 07:27 AM, Julian Foad wrote:
>> >>From IRC:
>> >
>> > julianf: We changed error codes in libsvn_wc all over the place.
>> > I don't think we see strict error codes as part of the documented
>> > behavior, unless it is in the function documentation
>>
>> I agree with this. Any time a specific error code is intended for use by an
>> API function as a specific message to the caller, we document that fact and
>> that behavior becomes part of the contract. Undocumented error codes are
>> fair game for changing over time.
>
> OK, then that's fine by me too.
>
> Noorul wrote:
>> Daniel Shahaf wrote:
>> > Why do you have to place the "svn: " prefix here explicitly?
>> Normally
>> > svn_handle_error2() would do that for you. This is a red flag ("is
>> > a wheel being reinvented here?") for me.
>>
>> We are actually consuming the error here to print it and proceed with
>> the other targets.
>
> Noorul, "svn_handle_error2" has a "fatal" flag that controls whether it
> terminates the program or not; setting fatal=FALSE would enable you to
> continue processing the other targets. But it prints "the error stack"
> which is not what you want here - we want just a single error message.
>
> Try using "svn_handle_warning2" instead. That function appears to do
> almost exactly what you want here; the only differences I can see is it
> inserts "warning: " before the message, which I think is perfect for
> this usage, and it doesn't print the extra "\n", which is easily
> rectified.
>
Please find updated patch attached. I used svn_handle_warning2 with
"svn: " as prefix because it is used in this fashion everywhere. All
tests pass with this patch.
Thanks and Regards
Noorul
Index: subversion/tests/cmdline/basic_tests.py
===
--- subversion/tests/cmdline/basic_tests.py (revision 1051915)
+++ subversion/tests/cmdline/basic_tests.py (working copy)
@@ -2260,7 +2260,8 @@
# Check for the correct error message
for line in errput:
-if re.match(".*\(Not a valid URL\).*", line):
+if re.match(".*" + idonotexist_url + ".*non-existent in revision 1.*",
+line):
return
# Else never matched the expected error output, so the test failed.
Index: subversion/svn/info-cmd.c
===
--- subversion/svn/info-cmd.c (revision 1051915)
+++ subversion/svn/info-cmd.c (working copy)
@@ -566,29 +566,14 @@
{
/* If one of the targets is a non-existent URL or wc-entry,
don't bail out. Just warn and move on to the next target. */
- if (err->apr_err == SVN_ERR_UNVERSIONED_RESOURCE
- || err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
+ if (err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND ||
+ err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
{
- SVN_ERR(svn_cmdline_fprintf
- (stderr, subpool,
- _("%s: (Not a versioned resource)\n\n"),
- svn_path_is_url(truepath)
- ? truepath
- : svn_dirent_local_style(truepath, pool)));
+ svn_handle_warning2(stderr, err, "svn: ");
+ svn_error_clear(svn_cmdline_fprintf(stderr, subpool, "\n"));
}
- else if (err->apr_err == SVN_ERR_RA_ILLEGAL_URL)
-{
- SVN_ERR(svn_cmdline_fprintf
- (stderr, subpool,
- _("%s: (Not a valid URL)\n\n"),
- svn_path_is_url(truepath)
- ? truepath
- : svn_dirent_local_style(truepath, pool)));
-}
else
-{
return svn_error_return(err);
-}
svn_error_clear(err);
err = NULL;
Re: [PATCH] Fix syntax inconsistency and update comments.
Noorul Islam K M writes: > "Hyrum K. Wright" writes: > >> On Wed, Dec 22, 2010 at 12:27 AM, Noorul Islam K M wrote: >> >>> >>> Find attached minor patch which makes 'if' syntax consistent and also >>> some comment improvements. >>> >>> Log >>> >>> [[[ >>> >>> Fix syntax inconsistency and update comments. >>> >>> * libsvn_client/locking_commands.c >>> (organize_lock_targets): Fix syntax inconsistency. Update comments. >> >> *What* about the syntax inconsistency are you fixing? White space? >> Function names? Use of braces? (I can tell by looking at the patch, >> but a more informative log message would be nice.) >> >> Same with "update comments". Update them in what way? >> > > Please find updated log message. > > [[[ > > Fix syntax inconsistency and update comments. > > * libsvn_client/locking_commands.c > (organize_lock_targets): Fix syntax inconsistency by removing > unnecessary braces from 'if' block. Update comments to reflect new > function names used. > > Patch by: Noorul Islam K M > ]]] > I was thinking that this one is a trivial fix. Thanks and Regards Noorul
RE: [PATCH] Fix some deprecation warnings
Thanks Vijay. Committed in r1052547. With regards Kamesh Jayachandran -Original Message- From: Vijayaguru Guruchave Sent: Fri 12/24/2010 9:42 PM To: dev@subversion.apache.org Cc: Daniel Shahaf; Kamesh Jayachandran Subject: Re: [PATCH] Fix some deprecation warnings On Friday 24 December 2010 03:46 PM, vijay wrote: > On Wednesday 22 December 2010 06:42 PM, Daniel Shahaf wrote: >> Kamesh Jayachandran wrote on Wed, Dec 22, 2010 at 15:10:46 +0530: >>> On 12/22/2010 02:02 AM, vijay wrote: Hi, I have attached a patch that fixes few deprecation warnings while compiling libsvn_client/log.c. Attached log message also. Thanks& Regards, Vijayaguru >>> Can you pass scratch_pool for the below call as 'iterpool' and move the >>> iterpool destruction down? >>> >> ... presumably in order to save a bit of memory. >> >> Daniel >> (not disagreeing) >> > I used iterpool in place of scratch pool and moved the iterpool > destruction down. There are two failures in log_tests.py:7 & 9 with > the following error_trace. > > > CMD: /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/svn/svn > log svn-test-work/working_copies/log_tests-7/A/B/E/b...@8 --config-dir > /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/tests/cmdline/svn-test-work/local_tmp/config > > --password rayjandom --no-auth-cache --username jrandom exited with 1 > > ../subversion/svn/log-cmd.c:743: (apr_err=22) > ../subversion/libsvn_client/log.c:486: (apr_err=22) > ../subversion/libsvn_client/ra.c:482: (apr_err=22) > ../subversion/libsvn_client/url.c:53: (apr_err=22) > ../subversion/libsvn_subr/dirent_uri.c:1667: (apr_err=22) > ../subversion/libsvn_subr/utf.c:837: (apr_err=22) > ../subversion/libsvn_subr/utf.c:604: (apr_err=22) > svn: Valid UTF-8 data > (hex:) > followed by invalid UTF-8 sequence > (hex: e0 65 30 00) > Traceback (most recent call last): > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", > > line 1212, in run > rc = self.pred.run(sandbox) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/testcase.py", > > line 170, in run > return self.func(sandbox) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/log_tests.py", > > line 762, in log_wc_with_peg_revision > 'log', my_path) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", > > line 264, in run_and_verify_svn > expected_exit, *varargs) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", > > line 302, in run_and_verify_svn2 > exit_code, out, err = main.run_svn(want_err, *varargs) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", > > line 580, in run_svn > *(_with_auth(_with_config_dir(varargs > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", > > line 338, in run_command > None, *varargs) > File > "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", > > line 513, in run_command_stdin > raise Failure > Failure > FAIL: log_tests.py 7: 'svn log wc_tar...@n' > > > The following function 'svn_uri_condense_targets()' stores its return > value of 'url_or_path' in scratch pool(here iterpool) which should not > be the case. > > SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets, > target_urls, TRUE, pool, iterpool)); > > We have to copy the return value from scratch pool to result pool > before exiting from the function, right? > I will send a patch to fix it in dirent_uri.c: > svn_uri_condense_targets(). > > Thanks & Regards, > vijayaguru The above mentioned bug is fixed in r1052505. Attaching the updated patch and log message. Thanks & Regards, Vijayaguru >>> + SVN_ERR(svn_uri_condense_targets(&url_or_path,&condensed_targets, >>> + target_urls, TRUE, pool, >>> pool)); >>> >>> >>> >>> >>> With regards >>> Kamesh Jayachandran >
Re: Safe to add a field to svn_error_t?
On 12/24/10 6:07 AM, Hyrum K Wright wrote:
On Fri, Dec 24, 2010 at 12:10 AM, Blair Zajac wrote:
The addition of "tracing" to svn_error_t in 1.7 currently uses a special
message string "traced call". Instead of doing this, can we safely add an
svn_boolean_t to svn_error_t the indicates if it is a traced error?
While svn_error_t is public, I'm hoping most people are creating them with
svn_error_create*(), so if that's the case, then code written and compiled
against svn<= 1.6 would work fine when linked against a 1.7 library.
Thoughts?
We don't explicitly say "only use approved methods to create this
object", but we are pretty implicit about it, with an entire
documentation group for "Error creation and destruction". I'm on the
fence on this, but would lean toward "go ahead and extend the struct"
if pressured.
What problem are you trying to solve?
There's this comment in svn_error__is_tracing_link():
svn_boolean_t
svn_error__is_tracing_link(svn_error_t *err)
{
#ifdef SVN_ERR__TRACING
/* ### A strcmp()? Really? I think it's the best we can do unless
### we add a boolean field to svn_error_t that's set only for
### these "placeholder error chain" items. Not such a bad idea,
### really... */
return (err && err->message && !strcmp(err->message, SVN_ERR__TRACED));
#else
return FALSE;
#endif
}
I thought of two other solutions:
1) a sentinel message and we compare by address.
2) negative source file line numbers.
Blair
Re: [PATCH] Fix some deprecation warnings
On Friday 24 December 2010 03:46 PM, vijay wrote: On Wednesday 22 December 2010 06:42 PM, Daniel Shahaf wrote: Kamesh Jayachandran wrote on Wed, Dec 22, 2010 at 15:10:46 +0530: On 12/22/2010 02:02 AM, vijay wrote: Hi, I have attached a patch that fixes few deprecation warnings while compiling libsvn_client/log.c. Attached log message also. Thanks& Regards, Vijayaguru Can you pass scratch_pool for the below call as 'iterpool' and move the iterpool destruction down? ... presumably in order to save a bit of memory. Daniel (not disagreeing) I used iterpool in place of scratch pool and moved the iterpool destruction down. There are two failures in log_tests.py:7 & 9 with the following error_trace. CMD: /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/svn/svn log svn-test-work/working_copies/log_tests-7/A/B/E/b...@8 --config-dir /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom exited with 1 ../subversion/svn/log-cmd.c:743: (apr_err=22) ../subversion/libsvn_client/log.c:486: (apr_err=22) ../subversion/libsvn_client/ra.c:482: (apr_err=22) ../subversion/libsvn_client/url.c:53: (apr_err=22) ../subversion/libsvn_subr/dirent_uri.c:1667: (apr_err=22) ../subversion/libsvn_subr/utf.c:837: (apr_err=22) ../subversion/libsvn_subr/utf.c:604: (apr_err=22) svn: Valid UTF-8 data (hex:) followed by invalid UTF-8 sequence (hex: e0 65 30 00) Traceback (most recent call last): File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 1212, in run rc = self.pred.run(sandbox) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/testcase.py", line 170, in run return self.func(sandbox) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/log_tests.py", line 762, in log_wc_with_peg_revision 'log', my_path) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", line 264, in run_and_verify_svn expected_exit, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", line 302, in run_and_verify_svn2 exit_code, out, err = main.run_svn(want_err, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 580, in run_svn *(_with_auth(_with_config_dir(varargs File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 338, in run_command None, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 513, in run_command_stdin raise Failure Failure FAIL: log_tests.py 7: 'svn log wc_tar...@n' The following function 'svn_uri_condense_targets()' stores its return value of 'url_or_path' in scratch pool(here iterpool) which should not be the case. SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets, target_urls, TRUE, pool, iterpool)); We have to copy the return value from scratch pool to result pool before exiting from the function, right? I will send a patch to fix it in dirent_uri.c: svn_uri_condense_targets(). Thanks & Regards, vijayaguru The above mentioned bug is fixed in r1052505. Attaching the updated patch and log message. Thanks & Regards, Vijayaguru + SVN_ERR(svn_uri_condense_targets(&url_or_path,&condensed_targets, + target_urls, TRUE, pool, pool)); With regards Kamesh Jayachandran [[[ Reslove some deprecation warnings using uri/dirent functions in its respective places. * subversion/libsvn_client/log.c: (svn_client_log5): Use 'svn_uri_condense_targets()' and 'svn_dirent_condense_targets()' * subversion/svn/commit-cmd.c: (svn_cl__commit): Use 'svn_dirent_condense_targets()' Patch by: Vijayaguru G Suggested by: kameshj Noorul Islam K M ]]] Index: subversion/libsvn_client/log.c === --- subversion/libsvn_client/log.c (revision 1052459) +++ subversion/libsvn_client/log.c (working copy) @@ -452,15 +452,14 @@ APR_ARRAY_PUSH(target_urls, const char *) = url; APR_ARRAY_PUSH(real_targets, const char *) = target; } - svn_pool_destroy(iterpool); /* if we have no valid target_urls, just exit. */ if (target_urls->nelts == 0) return SVN_NO_ERROR; /* Find the base URL and condensed targets relative to it. */ - SVN_ERR(svn_path_condense_targets(&url_or_path, &condensed_targets, -target_urls, TRUE, pool)); + SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets, + target_urls, TRUE, pool, iterpool)); if (condensed_targets->nelts == 0) APR_ARRAY_PUSH(conden
Re: [PATCH] svn info - changing hard coded error message to specific one.
On Fri, 2010-12-24, C. Michael Pilato wrote:
> On 12/23/2010 07:27 AM, Julian Foad wrote:
> >>From IRC:
> >
> > julianf: We changed error codes in libsvn_wc all over the place.
> > I don't think we see strict error codes as part of the documented
> > behavior, unless it is in the function documentation
>
> I agree with this. Any time a specific error code is intended for use by an
> API function as a specific message to the caller, we document that fact and
> that behavior becomes part of the contract. Undocumented error codes are
> fair game for changing over time.
OK, then that's fine by me too.
Noorul wrote:
> Daniel Shahaf wrote:
> > Why do you have to place the "svn: " prefix here explicitly?
> Normally
> > svn_handle_error2() would do that for you. This is a red flag ("is
> > a wheel being reinvented here?") for me.
>
> We are actually consuming the error here to print it and proceed with
> the other targets.
Noorul, "svn_handle_error2" has a "fatal" flag that controls whether it
terminates the program or not; setting fatal=FALSE would enable you to
continue processing the other targets. But it prints "the error stack"
which is not what you want here - we want just a single error message.
Try using "svn_handle_warning2" instead. That function appears to do
almost exactly what you want here; the only differences I can see is it
inserts "warning: " before the message, which I think is perfect for
this usage, and it doesn't print the extra "\n", which is easily
rectified.
- Julian
Re: [PATCH] svn info - changing hard coded error message to specific one.
On 12/23/2010 07:27 AM, Julian Foad wrote: >>From IRC: > > julianf: We changed error codes in libsvn_wc all over the place. > I don't think we see strict error codes as part of the documented > behavior, unless it is in the function documentation I agree with this. Any time a specific error code is intended for use by an API function as a specific message to the caller, we document that fact and that behavior becomes part of the contract. Undocumented error codes are fair game for changing over time. -- C. Michael Pilato CollabNet <> www.collab.net <> Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On 24.12.2010 14:25, Stefan Fuhrmann wrote: On 23.12.2010 03:44, Gavin Beau Baumanis wrote: Hi Johan, I was intrigued by your requirement to create a large file for testing. I remember from a really long time ago when I learnt C, that we used a specific algorithm for creating "natural" and "random" text. With some help from Mr.Google found out about Markov Chains that look promising - I can't remember if that was what I learned about or not - but it looks like it might be a prove helpful none the less. A little further Googlng and I found this specific post on stackoverflow. http://stackoverflow.com/questions/1037719/how-can-i-quickly-create-large-1gb-textbinary-files-with-natural-content No Idea if it is going to help you specifically or not... but there are quite a few ideas in the comments; * Obtain a copy of the first 100MB from wikipedia - for example. You might try some recent LINUX tar ball (~400MB). It should be * mainly but probably not entirely text * very close to typical real-world data (large config file sections, lots of source code, maybe some binary / UTF16 data) * accessible to everybody for independent testing etc. Just an idea ;) -- Stefan^2. ... you may import many versions (including the RCs) of it to form a deep history. -- Stefan^2.
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On 20.12.2010 02:43, Johan Corveleyn wrote:
On Wed, Dec 15, 2010 at 10:58 AM, Stefan Fuhrmann wrote:
On 15.12.2010 02:30, Stefan Fuhrmann wrote:
On 14.12.2010 23:35, Johan Corveleyn wrote:
Thoughts?
Two things you might try:
* introduce a local variable for afile[i]
* replace the for() loop with two nested ones, keeping
calls to other functions out of the hot spot:
for (i=0; i< file_len;)
That should read:
for (i=0; i< file_len; i++)
{
/* hot spot: */
for(; i< file_len; i++)
{
curFile = afile[i];
if (curFile->chunk==-1)
curFile->chunk = 0;
else if (curFile->curp != curFile->endp -1)
curFile->curp++;
else
break;
}
if (i< file_len)
{
/* the complex, rarely used stuff goes here */
}
}
Ok, I tried this, but it didn't really help. It's still only about as
fast as before. While the macro version is about 10% faster (I made a
cleaner macro version, only macro'ing the hotspot stuff, with a
function call to the complex, rarely used stuff).
For the inline version, I tried several variations (always with
APR_INLINE in the function signature, and with local variable for
file[i]): with or without the nested for loop, with or without the
complex stuff factored out to a separate function, ... it made no
difference.
Maybe I'm still doing something wrong, keeping the compiler from
inlining it (but I'm not really a compiler expert, maybe I have to add
some compiler options or something, ...). OTOH, I now have a macro
implementation which is quite readable (and which uses the do ...
while(0) construct - thanks Peter), and which does the trick. So maybe
I should simply stick with that option ...
The key factor here is that file_len is only 2.
Basically, that will make my optimization a
pessimization.
Assuming that for most calls, curp of *both*
files will be somewhere *between* BOF and
EOF, the unrolling the loop may help:
#define INCREMENT_POINTERS
/* special code for the common case. 10 .. 12 ticks per execution */
static APR_INLINE svn_boolean_t
quickly_increment_pointers(struct file_info *afile[])
{
struct file_info *file0 = afile[0];
struct file_info *file1 = afile[1];
if ((afile0->chunk != -1) && (afile1->chunk != -1))
{
/* suitable_type */ nextp0 = afile0->curp + 1;
/* suitable_type */ nextp1 = afile1->curp + 1;
if ((nextp0 != afile0->endp) && (nextp1 != afile1->endp))
{
afile0->curp = nextp0;
afile1->curp = nextp1;
return TRUE;
}
}
return FALSE;
}
/* slow code covering all special cases */
static svn_error_t*
slowly_increment_pointers(struct file_info *file[], apr_size_t file_len,
apr_pool_t *pool)
{
for (i = 0; i < file_len;i++)
...
}
/* maybe as macro: */
return ((file_len == 2) && quickly_increment_pointers (file))
? SVN_NO_ERROR
: slowly_increment_pointers(file, file_len, pool);
Minor remark on C style:
static APR_INLINE svn_error_t *
increment_pointers(struct file_info *file[], int file_len, apr_pool_t *pool)
file_len should be apr_size_t unless it can assume negative
values in which case it should be apr_ssize_t.That way, you
know for sure it will never overflow. Int is potentially too short
(although extrmly unlikely in this case) and it is signed
(often not appropriate for an index value).
One more thing that might help speed up your code. The fact
that the calling overhead of a single & simple function turns
out to be a significant contribution to the overall runtime tells
us that the execution is tightly CPU-bound and squeezing
some extra cycles out of it could help. Try
struct file_info file[] instead of struct file_info *file[]
i.e. array of structs instead of array of pointers. The boring
background is as follows.
file[index]->member
requires 2 cache accesses: reading the pointer form the
array (assuming that the array pointer itself and the index
value are already in some register) followed by an access
relative to that pointer:
/* assume file -> reg_0, index -> reg_1 */
(1.1) mem[reg_0 + 4 * reg_1] -> reg_2// content of file[index], i.e.
file_info pointer
(1.2) mem[reg_2 + member_offset] -> reg_3 // content of member element
The really bad part of this is that (1.2) depends on (1.1) and
must wait for the data to come in from the cache. On Corei7
this takes 4 ticks (on others like PPC it takes 2). So, while
you can issue 1 cache request per tick, the dependency
makes the whole thing take at least 5 ticks.
Using an array of structures this becomes
file[index].member
/* assume file -> reg_0, index -> reg_1 */
(2.1) reg_1 * 32 -> reg_2 // offset of struct [index], assume 32 ==
sizeof (file_info)
(2.2) mem[reg_0 + reg_2 + member_offset] -> reg_3 // content of member
element
Now this is only 2 ticks with (2.1) often being hidden, pre-
calculated elsewhere or entirely unnecessary (if index is
fixed).
A final problem with the array of pointers issue is that
you often cannot reuse the result of (1.1) because in C
a pointer can point any
Re: Safe to add a field to svn_error_t?
On Fri, Dec 24, 2010 at 12:10 AM, Blair Zajac wrote: > The addition of "tracing" to svn_error_t in 1.7 currently uses a special > message string "traced call". Instead of doing this, can we safely add an > svn_boolean_t to svn_error_t the indicates if it is a traced error? > > While svn_error_t is public, I'm hoping most people are creating them with > svn_error_create*(), so if that's the case, then code written and compiled > against svn <= 1.6 would work fine when linked against a 1.7 library. > > Thoughts? We don't explicitly say "only use approved methods to create this object", but we are pretty implicit about it, with an entire documentation group for "Error creation and destruction". I'm on the fence on this, but would lean toward "go ahead and extend the struct" if pressured. What problem are you trying to solve? -Hyrum
Re: [PATCH] Fix a bug in dirent_uri.c:svn_uri_condense_targets()
On Fri, Dec 24, 2010 at 4:33 AM, vijay wrote: > Hi, > > This patch fixes the bug reported in > http://svn.haxx.se/dev/archive-2010-12/0592.shtml . We need to copy the > value of PCOMMON from SCRATCH_POOL to RESULT_POOL since we are exiting early > when there is only one target uri to work on. > > Attached the patch and log message. Good catch. r1052505. -Hyrum
Re: [RFC] diff-optimizations-bytes branch: avoiding function call overhead (?)
On 23.12.2010 03:44, Gavin Beau Baumanis wrote: Hi Johan, I was intrigued by your requirement to create a large file for testing. I remember from a really long time ago when I learnt C, that we used a specific algorithm for creating "natural" and "random" text. With some help from Mr.Google found out about Markov Chains that look promising - I can't remember if that was what I learned about or not - but it looks like it might be a prove helpful none the less. A little further Googlng and I found this specific post on stackoverflow. http://stackoverflow.com/questions/1037719/how-can-i-quickly-create-large-1gb-textbinary-files-with-natural-content No Idea if it is going to help you specifically or not... but there are quite a few ideas in the comments; * Obtain a copy of the first 100MB from wikipedia - for example. You might try some recent LINUX tar ball (~400MB). It should be * mainly but probably not entirely text * very close to typical real-world data (large config file sections, lots of source code, maybe some binary / UTF16 data) * accessible to everybody for independent testing etc. Just an idea ;) -- Stefan^2.
[PATCH] Fix a bug in dirent_uri.c:svn_uri_condense_targets()
Hi,
This patch fixes the bug reported in
http://svn.haxx.se/dev/archive-2010-12/0592.shtml . We need to copy
the value of PCOMMON from SCRATCH_POOL to RESULT_POOL since we are
exiting early when there is only one target uri to work on.
Attached the patch and log message.
Thanks & Regards,
Vijayaguru
[[[
Copy *PCOMMON from SCRATCH_POOL to RESULT_POOL in svn_uri_condense_targets()
since we are exiting early when there is exactly one target to work on.
* subversion/libsvn_subr/dirent_uri.c:
(svn_uri_condense_targets): Copy *PCOMMON into RESULT_POOL for single
target case.
Patch by: Vijayaguru G
]]]
Index: subversion/libsvn_subr/dirent_uri.c
===
--- subversion/libsvn_subr/dirent_uri.c (revision 1052459)
+++ subversion/libsvn_subr/dirent_uri.c (working copy)
@@ -2144,6 +2144,7 @@
/* Early exit when there's only one uri to work on. */
if (targets->nelts == 1)
{
+ *pcommon = apr_pstrdup(result_pool, *pcommon);
if (pcondensed_targets)
*pcondensed_targets = apr_array_make(result_pool, 0,
sizeof(const char *));
Re: [PATCH] Fix some deprecation warnings
On Wednesday 22 December 2010 06:42 PM, Daniel Shahaf wrote: Kamesh Jayachandran wrote on Wed, Dec 22, 2010 at 15:10:46 +0530: On 12/22/2010 02:02 AM, vijay wrote: Hi, I have attached a patch that fixes few deprecation warnings while compiling libsvn_client/log.c. Attached log message also. Thanks& Regards, Vijayaguru Can you pass scratch_pool for the below call as 'iterpool' and move the iterpool destruction down? ... presumably in order to save a bit of memory. Daniel (not disagreeing) I used iterpool in place of scratch pool and moved the iterpool destruction down. There are two failures in log_tests.py:7 & 9 with the following error_trace. CMD: /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/svn/svn log svn-test-work/working_copies/log_tests-7/A/B/E/b...@8 --config-dir /home/vijayaguru/svn-sandbox/svn-trunk/vpath/subversion/tests/cmdline/svn-test-work/local_tmp/config --password rayjandom --no-auth-cache --username jrandom exited with 1 ../subversion/svn/log-cmd.c:743: (apr_err=22) ../subversion/libsvn_client/log.c:486: (apr_err=22) ../subversion/libsvn_client/ra.c:482: (apr_err=22) ../subversion/libsvn_client/url.c:53: (apr_err=22) ../subversion/libsvn_subr/dirent_uri.c:1667: (apr_err=22) ../subversion/libsvn_subr/utf.c:837: (apr_err=22) ../subversion/libsvn_subr/utf.c:604: (apr_err=22) svn: Valid UTF-8 data (hex:) followed by invalid UTF-8 sequence (hex: e0 65 30 00) Traceback (most recent call last): File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 1212, in run rc = self.pred.run(sandbox) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/testcase.py", line 170, in run return self.func(sandbox) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/log_tests.py", line 762, in log_wc_with_peg_revision 'log', my_path) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", line 264, in run_and_verify_svn expected_exit, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/actions.py", line 302, in run_and_verify_svn2 exit_code, out, err = main.run_svn(want_err, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 580, in run_svn *(_with_auth(_with_config_dir(varargs File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 338, in run_command None, *varargs) File "/home/vijayaguru/svn-sandbox/svn-trunk/subversion/tests/cmdline/svntest/main.py", line 513, in run_command_stdin raise Failure Failure FAIL: log_tests.py 7: 'svn log wc_tar...@n' The following function 'svn_uri_condense_targets()' stores its return value of 'url_or_path' in scratch pool(here iterpool) which should not be the case. SVN_ERR(svn_uri_condense_targets(&url_or_path, &condensed_targets, target_urls, TRUE, pool, iterpool)); We have to copy the return value from scratch pool to result pool before exiting from the function, right? I will send a patch to fix it in dirent_uri.c: svn_uri_condense_targets(). Thanks & Regards, vijayaguru + SVN_ERR(svn_uri_condense_targets(&url_or_path,&condensed_targets, + target_urls, TRUE, pool, pool)); With regards Kamesh Jayachandran
Re: svn_error_purge_tracing() and clearing the errors Re: svn commit: r1051763 - in /subversion/trunk/subversion: include/private/svn_repos_private.h libsvn_repos/commit.c mod_dav_svn/version.c
Blair Zajac wrote on Thu, Dec 23, 2010 at 17:09:31 -0800: > On 12/23/10 3:01 AM, Daniel Shahaf wrote: >> I've made a quick sketch --- looks good? > > The suggestions look good. > > We could add some more tests, checking the behavior with SVN_NO_ERROR > input and one where all errors are tracing, which should return an error > from SVN_ERR_ASSERT(). > I'm not sure about the 'assert' part (both due to the "testing that it has asserted" issue and due to the fact that it's mathematiclaly legitimaste to return an empty chain (aka SVN_NO_ERROR) in that case). As to the rest, feel free to commit some tests (whether based on my sketch or not); I'd rather not block you, and it might be a little while before I can get to it myself... > As an aside, there's a comment about adding an svn_boolean_t to > svn_error_t instead of using strcmp(), which could be done in a totally > separate commit.
How to recognize that an SVN_ERR_ASSERT() triggered Re: svn_error_purge_tracing() and clearing the errors Re: svn commit: r1051763 - in /subversion/trunk/subversion: include/private/svn_repos_private.
Daniel Shahaf wrote on Fri, Dec 24, 2010 at 10:48:24 +0200:
> Blair Zajac wrote on Thu, Dec 23, 2010 at 17:09:31 -0800:
> > On 12/23/10 3:01 AM, Daniel Shahaf wrote:
> >> I've made a quick sketch --- looks good?
> >
> > The suggestions look good.
> >
> > We could add some more tests, checking the behavior with SVN_NO_ERROR
> > input and one where all errors are tracing, which should return an error
> > from SVN_ERR_ASSERT().
> >
>
> I'm not sure about the 'assert' part (both due to the "testing that it
> has asserted" issue and due to the fact that it's mathematiclaly
> legitimaste to return an empty chain (aka SVN_NO_ERROR) in that case).
BTW, the malfunction API only promises an error in the category
SVN_ERR_MALFUNC_CATEGORY_START, so I have assumed we should have
a macro to decide whether an error code is in a given category:
#define SVN_ERROR_IN_CATEGORY(err, category) \
((category) == (((err) ? (err)->apr_err : 0) / SVN_ERR_CATEGORY_SIZE) *
SVN_ERR_CATEGORY_SIZE)
{
if (SVN_ERROR_IN_CATEGORY(err, SVN_ERR_MALFUNC_CATEGORY_START)) { ... }
}
