Opened 11 years ago

Closed 11 years ago

Last modified 11 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:
Has a Patch: no 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 11 years ago.
Archivable.diff (659 bytes) - added by shinta 11 years ago.
TextInput.diff (480 bytes) - added by shinta 11 years ago.

Download all attachments as: .zip

Change History (20)

comment:1 Changed 11 years ago by jackburton

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 Changed 11 years ago by shinta

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 Changed 11 years ago by shinta

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 Changed 11 years ago by axeld

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

comment:5 Changed 11 years ago by shinta

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?

Changed 11 years ago by shinta

Attachment: DigestCode2086.cpp added

comment:6 Changed 11 years ago by axeld

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 Changed 11 years ago by jackburton

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

comment:8 Changed 11 years ago by shinta

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?

Changed 11 years ago by shinta

Attachment: Archivable.diff added

comment:9 Changed 11 years ago by shinta

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 Changed 11 years ago by axeld

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 Changed 11 years ago by shinta

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.

Changed 11 years ago by shinta

Attachment: TextInput.diff added

comment:12 Changed 11 years ago by axeld

Status: newassigned

comment:13 Changed 11 years ago by axeld

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 Changed 11 years ago by shinta

Thank you for big changing!

comment:15 Changed 11 years ago by shinta

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 Changed 11 years ago by axeld

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 Changed 11 years ago by shinta

Today's build works well. Thank you.

Note: See TracTickets for help on using tickets.