Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-16 Thread Ivan Timofeev

On 17.02.2012 00:20, Kohei Yoshida wrote:

As far as I know those STL methods that return its element are
guaranteed to return a reference to the stored instance, not its copy.


Ah, great, I was not sure of that.

On 17.02.2012 01:01, Michael Meeks wrote:

I guess :-) I imagine the real problem here is that any kind of
operation on the stl container could then realloc the underlying memory
in some way, busting the reference / pointer you handed out elsewhere;
so it looks only transiently safe to me ;-)


I inspected the usages of that function - they look safe. However, the 
details are elusive for the users...


Thanks for the answers!

Best Regards,
Ivan

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-16 Thread Kohei Yoshida
On Thu, Feb 16, 2012 at 4:01 PM, Michael Meeks  wrote:
>
> On Thu, 2012-02-16 at 15:20 -0500, Kohei Yoshida wrote:
>> > Is it a safe code?
>> >
>> >      pForbiddenCharacters = &maMap[ nLanguage ];
>> >      return pForbiddenCharacters;
> ...
>> So, in theory, taking the address of a returned object which itself is a
>> reference to the instance stored in the container is very much safe.
>
>        I guess :-) I imagine the real problem here is that any kind of
> operation on the stl container could then realloc the underlying memory
> in some way, busting the reference / pointer you handed out elsewhere;
> so it looks only transiently safe to me ;-)

Of course.  If the content of the container changes all bets are off.
But I thought that was a given in this context (the same way all
associated iterators are invalid once the state of the container
changes).

My understanding of the question was, does the address of the object
point to the real object inside the container or a temporary object?
And my answer was it points to the real object inside the container.
I didn't mean to insinuate anything else. Sorry if I gave such
impression.

Kohei
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-16 Thread Michael Meeks

On Thu, 2012-02-16 at 15:20 -0500, Kohei Yoshida wrote:
> > Is it a safe code?
> > 
> >  pForbiddenCharacters = &maMap[ nLanguage ];
> >  return pForbiddenCharacters;
...
> So, in theory, taking the address of a returned object which itself is a
> reference to the instance stored in the container is very much safe.

I guess :-) I imagine the real problem here is that any kind of
operation on the stl container could then realloc the underlying memory
in some way, busting the reference / pointer you handed out elsewhere;
so it looks only transiently safe to me ;-)

Or perhaps I'm missing something,

ATB,

Michael.

-- 
michael.me...@suse.com  <><, Pseudo Engineer, itinerant idiot

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-16 Thread Kohei Yoshida
On Thu, 2012-02-16 at 10:48 +0400, Ivan Timofeev wrote:
> Whoa, I seem to be too naive for patch-reviewing...
> 
> Is it a safe code?
> 
>  pForbiddenCharacters = &maMap[ nLanguage ];
>  return pForbiddenCharacters;

Yeah I don't see anything wrong with it.

> Will pForbiddenCharacters point to the deleted object, after the return?

Nope.  As far as I know those STL methods that return its element are
guaranteed to return a reference to the stored instance, not its copy.
So, in theory, taking the address of a returned object which itself is a
reference to the instance stored in the container is very much safe.

The only exception is std::vector which lumps its internal boolean
values into bitfield, but by doing it it can no longer meet the
requirement of STL container which must return reference to its
elements.  So, by that definition std::vector is not a STL
container, strictly speaking.

For the curious, refer to Item 18 of Effective STL by Scott Meyers.

Kohei

-- 
Kohei Yoshida, LibreOffice hacker, Calc

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-15 Thread Ivan Timofeev

Whoa, I seem to be too naive for patch-reviewing...

Is it a safe code?

pForbiddenCharacters = &maMap[ nLanguage ];
return pForbiddenCharacters;

Will pForbiddenCharacters point to the deleted object, after the return?

Terribly sorry for such a question...

Regards,
Ivan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


Re: [PUSHED][PATCH] convert table.hxx use in editeng/inc/editeng/forbiddencharacterstable.hxx

2012-02-15 Thread Ivan Timofeev

Hi Noel,

On 15.02.2012 19:01, Noel Grandin wrote:

Updated patch implementing Ivan's suggestion. Passes make and make check.

Convert tools/table.hxx usage in
editeng/inc/editeng/forbiddencharacterstable.hxx to std::map


Looks good to me, pushed (without GetCharInfo - it was unused):
http://cgit.freedesktop.org/libreoffice/core/commit/?id=9d4b0c25a598a53601e2bd337443728f17f8296a
and tweaked:
http://cgit.freedesktop.org/libreoffice/core/commit/?id=843eafc765a3d1d0ea4c9a89855c73e81784aa8b
(I think, having 'Map' as a type alias is enough... :)

Thanks for your work!

Regards,
Ivan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice