Re: [PATCH] No hash table for load_props(), follow-up for r34630

On Wed, 2008-12-10 at 14:25 +0530, Bhuvaneswaran A wrote:
> On Wed, 2008-12-10 at 00:48 -0800, Greg Stein wrote:
> > s/You can just/You can't just/
> >
> > Look at the code you're changing here. Think about what it is doing
> > before writing a patch.
>
> I did run make && make check.
>
> /me looking into the code again.

Yes, the patch is definitely invalid. I did run make check before
submitting the patch, but instead of verifying the actual result i
verified the result from a file, assuming i redirected the test result
to that file.

Sorry about that mistake. Thank you for your time to review the patch
though!

> On Wed, Dec 10, 2008 at 00:47, Greg Stein <gstein_at_gmail.com> wrote:
> > > Euh... did you even *try* to run any portion of the test suite after
> > > this change? Really, this isn't going to work at all. I don't even
> > > have to try applying it.
> > >
> > > The callers need the hash table filled with the props. You can just
> > > make the hash table inside the function, then throw it away. All those
> > > callers won't end up with any data!!
> > >
> > > The hash table has to be an *out* parameter.
> > >
> > > -g
> > >
> > > On Wed, Dec 10, 2008 at 00:40, Bhuvaneswaran A <bhuvan_at_collab.net> wrote:
> > >> This is a follow-up patch for r34630 to not hash table to load_props()
> > >> function and avoid making hash before calling this function. The hash
> > >> table is created within the function.
> > >>
> > >> {{
> > >> This is a follow-up fix for r34630. Make hash table inside the
> > >> load_props() function, instead of passing it as an argument.
> > >>
> > >> Suggested by: gstein
> > >>
> > >> * subversion/libsvn_wc/props.c
> > >> (load_props): Remove the argument apr_hash_t *hash and define it
> > >> within the function.
> > >> (svn_wc__load_props, svn_wc__wcprop_list, modified_props): Adjust the
> > >> callers.
> > >> }}
> > >> --
> > >> Regards,
> > >> Bhuvaneswaran
> > >>
> > >> ------------------------------------------------------
> > >> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=982077
> > >
> --
> Regards,
> Bhuvaneswaran
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=982084

-- 
Regards,
Bhuvaneswaran
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=982100