Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#2086 closed bug (fixed)

BTextControl::BTextControl(BMessage* msg) causes segment violation

Reported by: shinta Owned by: axeld
Priority: normal Milestone: R1
Component: Kits/Interface Kit Version: R1/pre-alpha1
Keywords: Cc:
Blocked By: Blocking:
Platform: x86

Description

On creating BView with archived BMessage, if BMessage contains BTextControl object, the application causes segment violation at BTextView::SetAlignment().

Haiku: hrev24912 SHINTA

Attachments (3)

DigestCode2086.cpp (788 bytes ) - added by shinta 12 years ago.
Archivable.diff (659 bytes ) - added by shinta 12 years ago.
TextInput.diff (480 bytes ) - added by shinta 12 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 by jackburton, 12 years ago

From a quick look at BTextControl::Archive(), seems like the fText is never archived, so the unarchiving constructor doesn't find it in the BMessage.

comment:2 by shinta, 12 years ago

Thank you for your investigation.

I think that archived class name and instantiate class name are different.When I modified TextInput.cpp as follows, BTextControl::BTextcontrol(BMessage*) works well.

_BTextInput_::Instantiate(BMessage* archive) { if (validate_instantiation(archive, "BPrivate::_BTextInput_")); ...(omit)... }

Currently, archived BMessage has the class name "BPrivate::_BTextInput_". But, archived BMessage in ZETA (and maybe in BeOS, too) has the class name "_BTextInput_".

I think it is better to archive with the name "_BTextInput_" for backwards compatibility, then we don't need to modify _BTextInput_::Instantiate().

SHINTA

comment:3 by shinta, 12 years ago

I removed namespace "BPrivate" from BTextInput.cpp, then BTextInput was archived with the class name "_BTextInput_". It works well not only for Haiku's archive, but also for ZETA's archive.

Do you think removing namespace "BPrivate" is out of rule?

comment:4 by axeld, 12 years ago

IMO it should stay in the BPrivate namespace, if possible.

comment:5 by shinta, 12 years ago

To satisfy 2 conditions (backwards compatibility and staying namespace), I have an idea although it is ad-hoc.

I modified Archivable.cpp to convert "_BTextInput_" <-> "BPrivate::_BTextInput_". See DigestCode2086.cpp.

How about it?

by shinta, 12 years ago

Attachment: DigestCode2086.cpp added

comment:6 by axeld, 12 years ago

I would prefer a more generic approach, but that would of course work.

BTW according to our coding style, there are no blanks between the parentheses and their contents, ie. it would be: if (name == "...") rather than: if ( name == "..." )

comment:7 by jackburton, 12 years ago

But do we really need to keep the compatibility with the old name ?

comment:8 by shinta, 12 years ago

I heared Haiku keeps compatibility with R5 and I advocate it. I think that compatibility helps at least Haiku R1.

Ofcource, it is a way to throw compatibility although I'm sad.

Which is the project's policy, keep or throw compatibility?

by shinta, 12 years ago

Attachment: Archivable.diff added

comment:9 by shinta, 12 years ago

I uploaded a patch(Archivable.diff).

I don't like ad-hoc codes, too. But, I have no other idea.

Let's use this patch. When better code appears, this code will be overwritten.

comment:10 by axeld, 12 years ago

I'm not sure - since BMenu seems to have the same problem, I would rather tend to break compatibility there, unless there is a good reason. Wanting to take over your Tracker shelf from BeOS would not be a good reason, for example :-) If there is a usable app that actually comes with pre-archived interface elements, that would be a reason. In any case, it shouldn't crash either way.

comment:11 by shinta, 12 years ago

For example, some of my R5 app use pre-archived interface elements: Be88-BASIC, Light MIDI, CL-Lyrics Displayer, and so on. I don't know about other people's apps.

If you break compatibility, please use TextInput.diff. This patch avoid crash on unarchiving BTextControl archived in Haiku.

by shinta, 12 years ago

Attachment: TextInput.diff added

comment:12 by axeld, 12 years ago

Status: newassigned

comment:13 by axeld, 12 years ago

Resolution: fixed
Status: assignedclosed

Ah, those apps explain why you are interested in keeping compatibility, at least :-)

Fixed crash in hrev25123, added work-around for the compatibility issues in hrev25124. Hope that helps.

I've tested with Be88-BASIC, but the app still crashes for some reason (it obviously fails to initialize a BMenu variable, and then uses it as if nothing had happened). _BMCItem_ and _BTextInput_ should now be initialized correctly, though.

comment:14 by shinta, 12 years ago

Thank you for big changing!

comment:15 by shinta, 12 years ago

Changeset 25124 said:

If the class name starts with '_', it will now try to add a BPrivate namespace

in case it could not find the class. This should help with the compatibility issues Shinta reported (also part of ticket #2086).

Is it really try to add a BPrivate namespace?

I archived BTextControl in Haiku. The archive had "BPrivate::_BTextInput_". When I unarchived it, _BTextInput_::_BTextInput_(BRect...) constructor (not _BTextInput_::_BTextInput_(BMessage*)) was called.

comment:16 by axeld, 12 years ago

I have an idea about that - currently, we try to validate the object using _BTextInput_, but that would only work for old archives. I'll have a look.

comment:17 by shinta, 12 years ago

Today's build works well. Thank you.

Note: See TracTickets for help on using tickets.