Libpano locale fix

[Yuval Levy]

http://sourceforge.net/tracker/?func=detail&atid=550441&aid=2826516&group_id=77506

can somebody please apply the patch attached to Hugin's tracker to
Libpano? or even better: give Thomas Modes SVN write access to libpano?

Yuv

[D M German]

Hi Yuv,


I have looked at the patch and this is the improper way to deal with
this problem. It modifies a bunch of functions to reset the locale. It
is too cross-cutting. Have you looked at the patch?

It is basically ~40 insertions of this code:

+ setlocale(LC_ALL,old_locale);
+ free(old_locale);


I suspect it would be easier to reset the locale in hugin after the
panotools functions are called. that is just once place, compared to the
40 places that this patch would modify in libpano.

What we need is to make libpano support internationalization. This is
long overdue and might be a good project now that version 13 is out.

What do others think?

BTW, please reply to the panotools list.

Yuval> http://sourceforge.net/tracker/?func=detail&atid=550441&aid=2826516&group_id=77506

Yuval> can somebody please apply the patch attached to Hugin's tracker to
Yuval> Libpano? or even better: give Thomas Modes SVN write access to libpano?

Yuval> Yuv

Yuval>



--
Daniel M. German
http://turingmachine.org/
http://silvernegative.com/
dmg (at) uvic (dot) ca
replace (at) with @ and (dot) with .

[T. Modes]

Hi Daniel,

> I have looked at the patch and this is the improper way to deal with
> this problem. It modifies a bunch of functions to reset the locale. It
> is too cross-cutting. Have you looked at the patch?
>
> It is basically ~40 insertions of this code:
>
> + setlocale(LC_ALL,old_locale);
> + free(old_locale);
>

I don't agree. Libpano changes the locale in some function (call of
setlocale(LC_ALL,"C") mainly in parser.c), but does not restore the
old value at the end. This is no clean coding. When you change a
global setting in a function you should also restore the old value at
the end (this is the main function of the inserted lines).

> I suspect it would be easier to reset the locale in hugin after the
> panotools functions are called. that is just once place, compared to the
> 40 places that this patch would modify in libpano.

This would go, but it should only be a workaround.

Thomas

[dmg]

On Sat, Aug 1, 2009 at 1:32 AM, T. Modes<Thomas...@gmx.de> wrote:
>
> I don't agree. Libpano changes the locale in some function (call of
> setlocale(LC_ALL,"C") mainly in parser.c), but does not restore the
> old value at the end. This is no clean coding. When you change a
> global setting in a function you should also restore the old value at
> the end (this is the main function of the inserted lines).
>

I agree that libpano is wrong in doing this. But I rather remove the setlocale
than adding 40 setlocales in _every return_.

--
--dmg

---
Daniel M. German
http://turingmachine.org

[T. Modes]

> I agree that libpano is wrong in doing this. But I rather remove the setlocale
> than adding 40 setlocales in _every return_.

I don't know if this will always work. I think, the setlocale is added
for correct reading and write of floats (always with point as decimal
separator). When you remove the call of setlocale you will probably
end up with some files with points and other with comma as decimal
separator.
Or do you use a own function to handle to conversion of float to
string and vice versa with correct interpreation of decimal separator?

Thomas

[Alex Romosan]

"T. Modes" <Thomas...@gmx.de> writes:

the patch that got applied to parser.c to save and restore locales
causes hugin to crash. this is on debian unstable (linux):

*** glibc detected *** hugin: double free or corruption (fasttop): 0x0000000001e54670 ***
======= Backtrace: =========
/lib/libc.so.6[0x7feab7e076c8]
/lib/libc.so.6(cfree+0x76)[0x7feab7e091d6]
/usr/local/lib/libpano13.so.1(ParseScript+0x1ae4)[0x7feabd576e04]
/usr/local/lib/libhuginbase.so.0.0(_ZN9HuginBase6PTools19calcCtrlPointErrorsERNS_12PanoramaDataE+0x3fe)[0x7feabe1ab2de]
hugin(_ZN2PT18wxLoadPTProjectCmd7executeEv+0xfb0)[0x5872e0]
hugin(_ZN14CommandHistory10addCommandEPN7AppBase7CommandISsEEb+0x1f1)[0x51db91]
hugin(_ZN9MainFrame15LoadProjectFileERK8wxString+0x20f)[0x491f8f]
hugin(_ZN8huginApp6OnInitEv+0x258f)[0x472cbf]
/usr/lib/libwx_baseu-2.8.so.0(_Z7wxEntryRiPPw+0x23)[0x7feabb4223b3]
hugin(main+0x12)[0x46f442]
/lib/libc.so.6(__libc_start_main+0xe6)[0x7feab7db45c6]
hugin[0x46f109]

this happens when i try to open an old project, or after i load images
in hugin and try to run the optimizer. if i revert parser.c to
revision 1016 hugin works again.

--alex--

--
| I believe the moment is at hand when, by a paranoiac and active |
| advance of the mind, it will be possible (simultaneously with |
| automatism and other passive states) to systematize confusion |
| and thus to help to discredit completely the world of reality. |