Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 16 August 2002 1:26 am, John Levon wrote:
 On Fri, Aug 09, 2002 at 01:06:30PM +0100, Angus Leeming wrote:
   That's true. It usually involves some horrid BadDrawable, as if the
   execvp failure is still trying to draw a non-existent pixmap or
   something
 
  can I get you to investigate since I'm sort of busy and you have been
  experiencing the problem...

 I spent ten minutes looking and got lost in the graphics code. We are
 firing finishedconversion with bool parameter of true which is calling
 imageConverted in the cache, but I haven't the energy to find my way
 around this maze right now

I thought this code was now really easy to follow :-(
Angus

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 16, 2002 at 10:02:17AM +0100, Angus Leeming wrote:

  around this maze right now
 
 I thought this code was now really easy to follow :-(

it probably is ! I only spent ten minutes... I just can't work out where
we are trying to display some non-existent file 

john

-- 
Someone turn off the good idea tap; we're drowning here!
- Rusty Russell

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 16 August 2002 4:18 pm, John Levon wrote:
 On Fri, Aug 16, 2002 at 10:02:17AM +0100, Angus Leeming wrote:
   around this maze right now
 
  I thought this code was now really easy to follow :-(

 it probably is ! I only spent ten minutes... I just can't work out where
 we are trying to display some non-existent file

You mean load a file that doesn't exist?

GraphicsCacheItem.C:
void CacheItem::Impl::loadImage()
{
setStatus(Loading);
lyxerr[Debug::GRAPHICS]  Loading image.  endl;

image_ = Image::newImage();

cl_.disconnect();
cl_ = image_-finishedLoading.connect(
boost::bind(Impl::imageLoaded, this, _1));
image_-load(file_to_load_);
}

So you should be looking at the load() method in one of
GraphicsImageXPM.C
xformsImage.C
QLImage.C

and add an if (!FileExists()) check.

Angus

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 16 August 2002 1:26 am, John Levon wrote:
> On Fri, Aug 09, 2002 at 01:06:30PM +0100, Angus Leeming wrote:
> > > That's true. It usually involves some horrid BadDrawable, as if the
> > > execvp failure is still trying to draw a non-existent pixmap or
> > > something
> >
> > can I get you to investigate since I'm sort of busy and you have been
> > experiencing the problem...
>
> I spent ten minutes looking and got lost in the graphics code. We are
> firing finishedconversion with bool parameter of "true" which is calling
> imageConverted in the cache, but I haven't the energy to find my way
> around this maze right now

I thought this code was now really easy to follow :-(
Angus

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 16, 2002 at 10:02:17AM +0100, Angus Leeming wrote:

> > around this maze right now
> 
> I thought this code was now really easy to follow :-(

it probably is ! I only spent ten minutes... I just can't work out where
we are trying to display some non-existent file 

john

-- 
"Someone turn off the good idea tap; we're drowning here!"
- Rusty Russell

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 16 August 2002 4:18 pm, John Levon wrote:
> On Fri, Aug 16, 2002 at 10:02:17AM +0100, Angus Leeming wrote:
> > > around this maze right now
> >
> > I thought this code was now really easy to follow :-(
>
> it probably is ! I only spent ten minutes... I just can't work out where
> we are trying to display some non-existent file

You mean load a file that doesn't exist?

GraphicsCacheItem.C:
void CacheItem::Impl::loadImage()
{
setStatus(Loading);
lyxerr[Debug::GRAPHICS] << "Loading image." << endl;

image_ = Image::newImage();

cl_.disconnect();
cl_ = image_->finishedLoading.connect(
boost::bind(::imageLoaded, this, _1));
image_->load(file_to_load_);
}

So you should be looking at the load() method in one of
GraphicsImageXPM.C
xformsImage.C
QLImage.C

and add an if (!FileExists()) check.

Angus

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 01:06:30PM +0100, Angus Leeming wrote:

  That's true. It usually involves some horrid BadDrawable, as if the
  execvp failure is still trying to draw a non-existent pixmap or
  something
 
 can I get you to investigate since I'm sort of busy and you have been 
 experiencing the problem...

I spent ten minutes looking and got lost in the graphics code. We are
firing finishedconversion with bool parameter of true which is calling
imageConverted in the cache, but I haven't the energy to find my way
around this maze right now

regards
john
-- 
Someone turn off the good idea tap; we're drowning here!
- Rusty Russell

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 01:06:30PM +0100, Angus Leeming wrote:

> > That's true. It usually involves some horrid BadDrawable, as if the
> > execvp failure is still trying to draw a non-existent pixmap or
> > something
> 
> can I get you to investigate since I'm sort of busy and you have been 
> experiencing the problem...

I spent ten minutes looking and got lost in the graphics code. We are
firing finishedconversion with bool parameter of "true" which is calling
imageConverted in the cache, but I haven't the energy to find my way
around this maze right now

regards
john
-- 
"Someone turn off the good idea tap; we're drowning here!"
- Rusty Russell

Re: nasty eqns lead to crash

[Kornel Benko]

-BEGIN PGP SIGNED MESSAGE-

On Thursday 08 August 2002 15:06, Andre Poenitz wrote:
 On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
  Kornel Benko sent me this file because we thought that the crash was
  caused by the previewing. However, I can't load it at all with current,
  with or without previews enabled.

 It loads here. I only problem seems to be underscores in labels (again).
 I'll have a look at that.

 Andre'

It loads here now, after making some scripts executable.
(Yes, I am using the non-installed version from the src-directory.
 I have seen the mails, which pointed out this problem, but I didn't pay
enough attention to it. Sorry)

Kornel
- -- 
Kornel Benko
[EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: PGP 6.5.8

iQCVAwUBPVOLGLewfbDGmeqhAQHeWwQA4FX+UPz71j+g2bQpDOsoKxolik4+CzKZ
vaRZXusIYrmB5t1VSvcJAd8bHPCVngpKe0zMlv3tjDSmUP7HxHcoLkS7BPu/yJr7
cepkEHZi6wtscCHCNelw6pdkVIE0a42Bt9iLWoGn2oS5Eibb5IOdVzuz0FO3runM
WTJTr0+fHu4=
=US00
-END PGP SIGNATURE-

Re: nasty eqns lead to crash

[Martin Vermeer]

On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
 
 On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
 
  Kornel Benko sent me this file because we thought that the crash was caused 
  by the previewing. However, I can't load it at all with current, with or 
  without previews enabled. Since it is chock full of equations and little 
  else, I thought André might like a look ;-)
 
...
 
 ==9208==
 ==9208== Conditional jump or move depends on uninitialised value(s)
 ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
 ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
 ==9208==by 0x80C7A00: Counters::reset(basic_stringchar, 
string_char_traitschar, __default_alloc_templatetrue, 0  const ) 
(counters.C:206)
 ==9208==by 0x81387FB: LyXText::setCounter(Buffer const *, Paragraph *) const 
(text2.C:1235)
 ==9208==
 ==9208== Conditional jump or move depends on uninitialised value(s)
 ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
 ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
 ==9208==by 0x80C7AD0: Counters::copy(Counters , Counters , basic_stringchar, 
string_char_traitschar, __default_alloc_templatetrue, 0 
 const ) (counters.C:216)
 ==9208==by 0x81386A2: LyXText::setCounter(Buffer const *, Paragraph *) const 
(text2.C:1225)
 
 Andre ? Martin ?

Seems correct to me. The first dozen lines or so in LyXText::setCounter are: 
if a par has no previous(), it resets its counter array, otherwise it
copies the one of its predecessor. Should cover all cases.

Inside Counters::reset and copy, I don't see it either. What should be 
undefined? match, if it is the empty string? To my eyes, it looks OK.
Or is perhaps .find() not supposed to have an empty arg?

Martin
-- 
Martin Vermeer  [EMAIL PROTECTED]
Helsinki University of Technology 
Department of Surveying
P.O. Box 1200, FIN-02015 HUT, Finland
:wq



msg42278/pgp0.pgp
Description: PGP signature

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 10:39 am, Martin Vermeer wrote:
 On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
  On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
   Kornel Benko sent me this file because we thought that the crash was
   caused by the previewing. However, I can't load it at all with current,
   with or without previews enabled. Since it is chock full of equations
   and little else, I thought André might like a look ;-)

 ...

  ==9208==
  ==9208== Conditional jump or move depends on uninitialised value(s)
  ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
  ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
  ==9208==by 0x80C7A00: Counters::reset(basic_stringchar,
  string_char_traitschar, __default_alloc_templatetrue, 0  const )
  (counters.C:206) ==9208==by 0x81387FB: LyXText::setCounter(Buffer
  const *, Paragraph *) const (text2.C:1235) ==9208==
  ==9208== Conditional jump or move depends on uninitialised value(s)
  ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
  ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
  ==9208==by 0x80C7AD0: Counters::copy(Counters , Counters ,
  basic_stringchar, string_char_traitschar,
  __default_alloc_templatetrue, 0  const ) (counters.C:216)
  ==9208==by 0x81386A2: LyXText::setCounter(Buffer const *, Paragraph
  *) const (text2.C:1225)
 
  Andre ? Martin ?

 Seems correct to me. The first dozen lines or so in LyXText::setCounter
 are: if a par has no previous(), it resets its counter array, otherwise it
 copies the one of its predecessor. Should cover all cases.

 Inside Counters::reset and copy, I don't see it either. What should be
 undefined? match, if it is the empty string? To my eyes, it looks OK.
 Or is perhaps .find() not supposed to have an empty arg?

It doesn't, it has a string argument that just happens to be empty. That's 
fine and string::find will return string::npos (for which you already test).

That is assuming that you haven't populated your map with an empty string() 
as an index...

Incidentally, why not:

void Counters::reset(string const  match)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
if (match.empty()) {
for (; it != end; ++it) {
it-second.reset();
}
} else {
for (; it != end; ++it) {
if (it-first.find(match) != string::npos)
it-second.reset();
}
}
}

And whether you choose to use two loops or one, that should be match.empty(), 
not match == . 

If you choose to remain with a single loop then you should also move the 
match.empty() test to the front of the if statement as it's quicker than 
find... (I believe that they're executed in order).

Angus

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 10:37 am, Angus Leeming wrote:

 Incidentally, why not:

 void Counters::reset(string const  match)
 {
   CounterList::iterator it = counterList.begin();
   CounterList::iterator end = counterList.end();
   if (match.empty()) {
   for (; it != end; ++it) {
   it-second.reset();
   }
   } else {
   for (; it != end; ++it) {
   if (it-first.find(match) != string::npos)
   it-second.reset();
   }
   }
 }

Actually, shouldn't that be:

void Counters::reset(string const  match)
{
if (match.empty()) {
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it-second.reset();
}
} else {
CounterList::iterator it = counterList.find(match);
if (it != counterList.end())
it-second.reset();
}
}

?

Angus

Re: nasty eqns lead to crash

[Andre Poenitz]

On Fri, Aug 09, 2002 at 10:37:10AM +0100, Angus Leeming wrote:
 (I believe that they're executed in order).

They have to.

Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| On Friday 09 August 2002 10:37 am, Angus Leeming wrote:

 Incidentally, why not:

 void Counters::reset(string const  match)
 {
  CounterList::iterator it = counterList.begin();
  CounterList::iterator end = counterList.end();
  if (match.empty()) {
  for (; it != end; ++it) {
  it-second.reset();
  }
  } else {
  for (; it != end; ++it) {
  if (it-first.find(match) != string::npos)
  it-second.reset();
  }
  }
 }

| Actually, shouldn't that be:

| void Counters::reset(string const  match)
| {
|   if (match.empty()) {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   for (; it != end; ++it) {
|   it-second.reset();
|   }
|   } else {
|   CounterList::iterator it = counterList.find(match);

Only if:
 - exact match is wanted.
 - only one element in counterList can match.

|   if (it != counterList.end())
|   it-second.reset();
|   }
| }

| ?

| Angus


-- 
Lgb

Re: nasty eqns lead to crash

[Martin Vermeer]

On Fri, Aug 09, 2002 at 10:37:10AM +0100, Angus Leeming wrote:

...

   Andre ? Martin ?
 
  Seems correct to me. The first dozen lines or so in LyXText::setCounter
  are: if a par has no previous(), it resets its counter array, otherwise it
  copies the one of its predecessor. Should cover all cases.
 
  Inside Counters::reset and copy, I don't see it either. What should be
  undefined? match, if it is the empty string? To my eyes, it looks OK.
  Or is perhaps .find() not supposed to have an empty arg?
 
 It doesn't, it has a string argument that just happens to be empty. That's 
 fine and string::find will return string::npos (for which you already test).

Unfortunately it didn't work properly. That's why I added the separate
empty test. (It returned haphazard, random-looking boolean values for

it-first.find(match) != string::npos

in case match was empty. That's gcc-2.95.2 on intel-linux.)
 
 That is assuming that you haven't populated your map with an empty string() 
 as an index...

Have I? :-)
 
...

 And whether you choose to use two loops or one, that should be match.empty(), 
 not match == . 

Be my guest.
 
 If you choose to remain with a single loop then you should also move the 
 match.empty() test to the front of the if statement as it's quicker than 
 find... (I believe that they're executed in order).

Yes, I agree.
 
 Angus
 

Martin




msg42284/pgp0.pgp
Description: PGP signature

Re: nasty eqns lead to crash

[Angus Leeming]

Amazing what happens if you dig deeper

aleem@pneumon:src- grep counters() *.C */*.C | grep copy
text2.C:par-counters().copy(par-previous()-counters(), 
par-counters(), );
aleem@pneumon:src- grep counters() *.C */*.C | grep reset
text2.C:par-counters().reset();
text2.C:par-counters().reset();
text2.C:par-counters().reset(enum);
text2.C:par-counters().reset(enum);

So, Counters::copy() is used only as 
Counters::copy(Counters  from, Counters  to);

and can therefore be simplified to

void Counters::copy(Counters  from, Counters  to)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
to.set(it-first, from.value(it-first));
} 
}


whilst Counters::reset should be written as

void Counters::reset(string const  match)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
if (match.empty()) {
for (; it != end; ++it) {
it-second.reset();
}
} else {
// reset all counters whose index contains match as a
// sub-string
for (; it != end; ++it) {
if (it-first.find(match) != string::npos)
it-second.reset();
}
}
}

Angus

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| Amazing what happens if you dig deeper

| aleem@pneumon:src- grep counters() *.C */*.C | grep copy
| text2.C:par-counters().copy(par-previous()-counters(), 
| par-counters(), );
| aleem@pneumon:src- grep counters() *.C */*.C | grep reset
| text2.C:par-counters().reset();
| text2.C:par-counters().reset();
| text2.C:par-counters().reset(enum);
| text2.C:par-counters().reset(enum);

| So, Counters::copy() is used only as 
|   Counters::copy(Counters  from, Counters  to);

| and can therefore be simplified to

| void Counters::copy(Counters  from, Counters  to)
| {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   for (; it != end; ++it) {
|   to.set(it-first, from.value(it-first));
|   } 
| }


| whilst Counters::reset should be written as

| void Counters::reset(string const  match)
| {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   if (match.empty()) {
|   for (; it != end; ++it) {
|   it-second.reset();
|   }
|   } else {
|   // reset all counters whose index contains match as a
|   // sub-string
|   for (; it != end; ++it) {
|   if (it-first.find(match) != string::npos)
|   it-second.reset();
|   }
|   }
| }

Hmm I wonder if this reset function shouldn't be split into two
functions...


void Counters::reset()
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it-second.reset();
}
}

Which could be simplified...

something similar to:

void Counters::reset()
{
std::for_each(counterList.begin(), counterList.end(),
  std::memfun(Counter::reset));
}


and

void Counters::reset(string const  match)
{
assert(!match.empty());

CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
if (it-first.find(match) != string::npos)
it-second.reset();
}
}


Functions that do only one thing and that does not decide what to do
based on the value of arguments are a good thing.

-- 
Lgb

Re: nasty eqns lead to crash

[Andre Poenitz]

On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
 and can therefore be simplified to
 
 void Counters::copy(Counters  from, Counters  to)
 {
   CounterList::iterator it = counterList.begin();
   CounterList::iterator end = counterList.end();
   for (; it != end; ++it) {
   to.set(it-first, from.value(it-first));
   } 
 }

Does this change 'from'?


Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 12:23 pm, Andre Poenitz wrote:
 On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
  and can therefore be simplified to
 
  void Counters::copy(Counters  from, Counters  to)
  {
  CounterList::iterator it = counterList.begin();
  CounterList::iterator end = counterList.end();
  for (; it != end; ++it) {
  to.set(it-first, from.value(it-first));
  }
  }

 Does this change 'from'?

int Counters::value(string const  ctr) const
{
CounterList::const_iterator cit = counterList.find(ctr);
if (cit == counterList.end()) {
lyxerr  value: Counter does not exist:   ctr  endl;
return 0;
}
return cit-second.value();
}

No.

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 11:59 am, Angus Leeming wrote:
 On Friday 09 August 2002 12:23 pm, Andre Poenitz wrote:
  On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
   and can therefore be simplified to
  
   void Counters::copy(Counters  from, Counters  to)
   {
 CounterList::iterator it = counterList.begin();
 CounterList::iterator end = counterList.end();
 for (; it != end; ++it) {
 to.set(it-first, from.value(it-first));
 }
   }
 
  Does this change 'from'?

 int Counters::value(string const  ctr) const
 {
   CounterList::const_iterator cit = counterList.find(ctr);
   if (cit == counterList.end()) {
   lyxerr  value: Counter does not exist:   ctr  endl;
   return 0;
   }
   return cit-second.value();
 }

 No.

But you are right to notice that the semantics of this method are very 
confusing.

1. It's a member of Counters, but does not set *this.

Is this what is meant:
void Counters::copy(Counters const  from)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it-set(from.value(it-first));
}
}

?

2. Alternatively, it could be a non-member function

void Counters::copy(Counters const  from, Counters  to)
{
CounterList::iterator it = to.counterList.begin();
CounterList::iterator end = to.counterList.end();
for (; it != end; ++it) {
it-set(from.value(it-first));
}
}

As it is, I have no idea what it is meant to be doing, but it ain't doing it!

Angus


But is Counters::copy doing what it's meant to be doing?

Why is it a member variable that resets to

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
 2. Alternatively, it could be a non-member function

 void Counters::copy(Counters const  from, Counters  to)

That's 
void copy(Counters const  from, Counters  to)

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 11:27:52AM +0200, Kornel Benko wrote:

 It loads here now, after making some scripts executable.
 (Yes, I am using the non-installed version from the src-directory.

anyone remember how we get them +x in the repository ? Do we have to
re-add them or is there a cvs thingy ?

regards
john

-- 
It is unbecoming for young men to utter maxims.
- Aristotle

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:18 pm, John Levon wrote:
 On Fri, Aug 09, 2002 at 11:27:52AM +0200, Kornel Benko wrote:
  It loads here now, after making some scripts executable.
  (Yes, I am using the non-installed version from the src-directory.

 anyone remember how we get them +x in the repository ? Do we have to
 re-add them or is there a cvs thingy ?

either way, LyX shouldn't crash...

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming [EMAIL PROTECTED] writes:

| On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
 2. Alternatively, it could be a non-member function

 void Counters::copy(Counters const  from, Counters  to)

| That's 
| void copy(Counters const  from, Counters  to)

How is this then different from a member function

void operator=(Counters const  from) ??

-- 
Lgb

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 12:51:03PM +0100, Angus Leeming wrote:

 either way, LyX shouldn't crash...

That's true. It usually involves some horrid BadDrawable, as if the
execvp failure is still trying to draw a non-existent pixmap or
something

john

-- 
It is unbecoming for young men to utter maxims.
- Aristotle

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:30 pm, Lars Gullik Bjønnes wrote:
 Angus Leeming [EMAIL PROTECTED] writes:
 | On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
  2. Alternatively, it could be a non-member function
 
  void Counters::copy(Counters const  from, Counters  to)
 |
 | That's
 | void copy(Counters const  from, Counters  to)

 How is this then different from a member function

 void operator=(Counters const  from) ??

Well that would copy from.counterList to this-counterList which is different 
from what I believe it is meant to be doing:

void Counters::copy(Counters const  from)
{
CounterList::iterator it  = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it-second.set(from.value(it-first));
}
}

Anyway, patch to follow.

Angus

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:30 pm, John Levon wrote:
 On Fri, Aug 09, 2002 at 12:51:03PM +0100, Angus Leeming wrote:
  either way, LyX shouldn't crash...

 That's true. It usually involves some horrid BadDrawable, as if the
 execvp failure is still trying to draw a non-existent pixmap or
 something

can I get you to investigate since I'm sort of busy and you have been 
experiencing the problem...

A

Re: nasty eqns lead to crash

[Kornel Benko]

-BEGIN PGP SIGNED MESSAGE-

On Thursday 08 August 2002 15:06, Andre Poenitz wrote:
> On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
> > Kornel Benko sent me this file because we thought that the crash was
> > caused by the previewing. However, I can't load it at all with current,
> > with or without previews enabled.
>
> It loads here. I only problem seems to be underscores in labels (again).
> I'll have a look at that.
>
> Andre'

It loads here now, after making some scripts executable.
(Yes, I am using the non-installed version from the src-directory.
 I have seen the mails, which pointed out this problem, but I didn't pay
enough attention to it. Sorry)

Kornel
- -- 
Kornel Benko
[EMAIL PROTECTED]
-BEGIN PGP SIGNATURE-
Version: PGP 6.5.8

iQCVAwUBPVOLGLewfbDGmeqhAQHeWwQA4FX+UPz71j+g2bQpDOsoKxolik4+CzKZ
vaRZXusIYrmB5t1VSvcJAd8bHPCVngpKe0zMlv3tjDSmUP7HxHcoLkS7BPu/yJr7
cepkEHZi6wtscCHCNelw6pdkVIE0a42Bt9iLWoGn2oS5Eibb5IOdVzuz0FO3runM
WTJTr0+fHu4=
=US00
-END PGP SIGNATURE-

Re: nasty eqns lead to crash

[Martin Vermeer]

On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
> 
> On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
> 
> > Kornel Benko sent me this file because we thought that the crash was caused 
> > by the previewing. However, I can't load it at all with current, with or 
> > without previews enabled. Since it is chock full of equations and little 
> > else, I thought André might like a look ;-)
 
...
 
> ==9208==
> ==9208== Conditional jump or move depends on uninitialised value(s)
> ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
> ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
> ==9208==by 0x80C7A00: Counters::reset(basic_stringstring_char_traits, __default_alloc_template > const &) 
>(counters.C:206)
> ==9208==by 0x81387FB: LyXText::setCounter(Buffer const *, Paragraph *) const 
>(text2.C:1235)
> ==9208==
> ==9208== Conditional jump or move depends on uninitialised value(s)
> ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
> ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
> ==9208==by 0x80C7AD0: Counters::copy(Counters &, Counters &, basic_stringstring_char_traits, __default_alloc_template >
> const &) (counters.C:216)
> ==9208==by 0x81386A2: LyXText::setCounter(Buffer const *, Paragraph *) const 
>(text2.C:1225)
> 
> Andre ? Martin ?

Seems correct to me. The first dozen lines or so in LyXText::setCounter are: 
if a par has no previous(), it resets its counter array, otherwise it
copies the one of its predecessor. Should cover all cases.

Inside Counters::reset and copy, I don't see it either. What should be 
undefined? match, if it is the empty string? To my eyes, it looks OK.
Or is perhaps .find() not supposed to have an empty arg?

Martin
-- 
Martin Vermeer  [EMAIL PROTECTED]
Helsinki University of Technology 
Department of Surveying
P.O. Box 1200, FIN-02015 HUT, Finland
:wq



msg42278/pgp0.pgp
Description: PGP signature

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 10:39 am, Martin Vermeer wrote:
> On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
> > On Thu, Aug 08, 2002 at 01:26:32PM +0100, Angus Leeming wrote:
> > > Kornel Benko sent me this file because we thought that the crash was
> > > caused by the previewing. However, I can't load it at all with current,
> > > with or without previews enabled. Since it is chock full of equations
> > > and little else, I thought André might like a look ;-)
>
> ...
>
> > ==9208==
> > ==9208== Conditional jump or move depends on uninitialised value(s)
> > ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
> > ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
> > ==9208==by 0x80C7A00: Counters::reset(basic_string > string_char_traits, __default_alloc_template > const &)
> > (counters.C:206) ==9208==by 0x81387FB: LyXText::setCounter(Buffer
> > const *, Paragraph *) const (text2.C:1235) ==9208==
> > ==9208== Conditional jump or move depends on uninitialised value(s)
> > ==9208==at 0x80C94FD: ??? (/usr/include/g++-3/std/bastring.cc:249)
> > ==9208==by 0x80C907B: ??? (/usr/include/g++-3/std/bastring.h:352)
> > ==9208==by 0x80C7AD0: Counters::copy(Counters &, Counters &,
> > basic_string > __default_alloc_template > const &) (counters.C:216)
> > ==9208==by 0x81386A2: LyXText::setCounter(Buffer const *, Paragraph
> > *) const (text2.C:1225)
> >
> > Andre ? Martin ?
>
> Seems correct to me. The first dozen lines or so in LyXText::setCounter
> are: if a par has no previous(), it resets its counter array, otherwise it
> copies the one of its predecessor. Should cover all cases.
>
> Inside Counters::reset and copy, I don't see it either. What should be
> undefined? match, if it is the empty string? To my eyes, it looks OK.
> Or is perhaps .find() not supposed to have an empty arg?

It doesn't, it has a string argument that just happens to be empty. That's 
fine and string::find will return string::npos (for which you already test).

That is assuming that you haven't populated your map with an empty string() 
as an index...

Incidentally, why not:

void Counters::reset(string const & match)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
if (match.empty()) {
for (; it != end; ++it) {
it->second.reset();
}
} else {
for (; it != end; ++it) {
if (it->first.find(match) != string::npos)
it->second.reset();
}
}
}

And whether you choose to use two loops or one, that should be match.empty(), 
not match == "". 

If you choose to remain with a single loop then you should also move the 
match.empty() test to the front of the if statement as it's quicker than 
find... (I believe that they're executed in order).

Angus

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 10:37 am, Angus Leeming wrote:

> Incidentally, why not:
>
> void Counters::reset(string const & match)
> {
>   CounterList::iterator it = counterList.begin();
>   CounterList::iterator end = counterList.end();
>   if (match.empty()) {
>   for (; it != end; ++it) {
>   it->second.reset();
>   }
>   } else {
>   for (; it != end; ++it) {
>   if (it->first.find(match) != string::npos)
>   it->second.reset();
>   }
>   }
> }

Actually, shouldn't that be:

void Counters::reset(string const & match)
{
if (match.empty()) {
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it->second.reset();
}
} else {
CounterList::iterator it = counterList.find(match);
if (it != counterList.end())
it->second.reset();
}
}

?

Angus

Re: nasty eqns lead to crash

[Andre Poenitz]

On Fri, Aug 09, 2002 at 10:37:10AM +0100, Angus Leeming wrote:
> (I believe that they're executed in order).

They have to.

Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| On Friday 09 August 2002 10:37 am, Angus Leeming wrote:
>
>> Incidentally, why not:
>>
>> void Counters::reset(string const & match)
>> {
>>  CounterList::iterator it = counterList.begin();
>>  CounterList::iterator end = counterList.end();
>>  if (match.empty()) {
>>  for (; it != end; ++it) {
>>  it->second.reset();
>>  }
>>  } else {
>>  for (; it != end; ++it) {
>>  if (it->first.find(match) != string::npos)
>>  it->second.reset();
>>  }
>>  }
>> }
>
| Actually, shouldn't that be:
>
| void Counters::reset(string const & match)
| {
|   if (match.empty()) {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   for (; it != end; ++it) {
|   it->second.reset();
|   }
|   } else {
|   CounterList::iterator it = counterList.find(match);

Only if:
 - exact match is wanted.
 - only one element in counterList can match.

|   if (it != counterList.end())
|   it->second.reset();
|   }
| }
>
| ?
>
| Angus
>

-- 
Lgb

Re: nasty eqns lead to crash

[Martin Vermeer]

On Fri, Aug 09, 2002 at 10:37:10AM +0100, Angus Leeming wrote:

...

> > > Andre ? Martin ?
> >
> > Seems correct to me. The first dozen lines or so in LyXText::setCounter
> > are: if a par has no previous(), it resets its counter array, otherwise it
> > copies the one of its predecessor. Should cover all cases.
> >
> > Inside Counters::reset and copy, I don't see it either. What should be
> > undefined? match, if it is the empty string? To my eyes, it looks OK.
> > Or is perhaps .find() not supposed to have an empty arg?
> 
> It doesn't, it has a string argument that just happens to be empty. That's 
> fine and string::find will return string::npos (for which you already test).

Unfortunately it didn't work properly. That's why I added the separate
empty test. (It returned haphazard, random-looking boolean values for

it->first.find(match) != string::npos

in case match was empty. That's gcc-2.95.2 on intel-linux.)
 
> That is assuming that you haven't populated your map with an empty string() 
> as an index...

Have I? :-)
 
...

> And whether you choose to use two loops or one, that should be match.empty(), 
> not match == "". 

Be my guest.
 
> If you choose to remain with a single loop then you should also move the 
> match.empty() test to the front of the if statement as it's quicker than 
> find... (I believe that they're executed in order).

Yes, I agree.
 
> Angus
> 

Martin




msg42284/pgp0.pgp
Description: PGP signature

Re: nasty eqns lead to crash

[Angus Leeming]

Amazing what happens if you dig deeper

aleem@pneumon:src-> grep "counters()" *.C */*.C | grep copy
text2.C:par->counters().copy(par->previous()->counters(), 
par->counters(), "");
aleem@pneumon:src-> grep "counters()" *.C */*.C | grep reset
text2.C:par->counters().reset("");
text2.C:par->counters().reset("");
text2.C:par->counters().reset("enum");
text2.C:par->counters().reset("enum");

So, Counters::copy() is used only as 
Counters::copy(Counters & from, Counters & to);

and can therefore be simplified to

void Counters::copy(Counters & from, Counters & to)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
to.set(it->first, from.value(it->first));
} 
}


whilst Counters::reset should be written as

void Counters::reset(string const & match)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
if (match.empty()) {
for (; it != end; ++it) {
it->second.reset();
}
} else {
// reset all counters whose index contains match as a
// sub-string
for (; it != end; ++it) {
if (it->first.find(match) != string::npos)
it->second.reset();
}
}
}

Angus

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| Amazing what happens if you dig deeper
>
| aleem@pneumon:src-> grep "counters()" *.C */*.C | grep copy
| text2.C:par->counters().copy(par->previous()->counters(), 
| par->counters(), "");
| aleem@pneumon:src-> grep "counters()" *.C */*.C | grep reset
| text2.C:par->counters().reset("");
| text2.C:par->counters().reset("");
| text2.C:par->counters().reset("enum");
| text2.C:par->counters().reset("enum");
>
| So, Counters::copy() is used only as 
|   Counters::copy(Counters & from, Counters & to);
>
| and can therefore be simplified to
>
| void Counters::copy(Counters & from, Counters & to)
| {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   for (; it != end; ++it) {
|   to.set(it->first, from.value(it->first));
|   } 
| }
>
>
| whilst Counters::reset should be written as
>
| void Counters::reset(string const & match)
| {
|   CounterList::iterator it = counterList.begin();
|   CounterList::iterator end = counterList.end();
|   if (match.empty()) {
|   for (; it != end; ++it) {
|   it->second.reset();
|   }
|   } else {
|   // reset all counters whose index contains match as a
|   // sub-string
|   for (; it != end; ++it) {
|   if (it->first.find(match) != string::npos)
|   it->second.reset();
|   }
|   }
| }

Hmm I wonder if this reset function shouldn't be split into two
functions...


void Counters::reset()
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it->second.reset();
}
}

Which could be simplified...

something similar to:

void Counters::reset()
{
std::for_each(counterList.begin(), counterList.end(),
  std::memfun(::reset));
}


and

void Counters::reset(string const & match)
{
assert(!match.empty());

CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
if (it->first.find(match) != string::npos)
it->second.reset();
}
}


Functions that do only one thing and that does not decide what to do
based on the value of arguments are a good thing.

-- 
Lgb

Re: nasty eqns lead to crash

[Andre Poenitz]

On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
> and can therefore be simplified to
> 
> void Counters::copy(Counters & from, Counters & to)
> {
>   CounterList::iterator it = counterList.begin();
>   CounterList::iterator end = counterList.end();
>   for (; it != end; ++it) {
>   to.set(it->first, from.value(it->first));
>   } 
> }

Does this change 'from'?


Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 12:23 pm, Andre Poenitz wrote:
> On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
> > and can therefore be simplified to
> >
> > void Counters::copy(Counters & from, Counters & to)
> > {
> > CounterList::iterator it = counterList.begin();
> > CounterList::iterator end = counterList.end();
> > for (; it != end; ++it) {
> > to.set(it->first, from.value(it->first));
> > }
> > }
>
> Does this change 'from'?

int Counters::value(string const & ctr) const
{
CounterList::const_iterator cit = counterList.find(ctr);
if (cit == counterList.end()) {
lyxerr << "value: Counter does not exist: " << ctr << endl;
return 0;
}
return cit->second.value();
}

No.

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 11:59 am, Angus Leeming wrote:
> On Friday 09 August 2002 12:23 pm, Andre Poenitz wrote:
> > On Fri, Aug 09, 2002 at 11:36:13AM +0100, Angus Leeming wrote:
> > > and can therefore be simplified to
> > >
> > > void Counters::copy(Counters & from, Counters & to)
> > > {
> > >   CounterList::iterator it = counterList.begin();
> > >   CounterList::iterator end = counterList.end();
> > >   for (; it != end; ++it) {
> > >   to.set(it->first, from.value(it->first));
> > >   }
> > > }
> >
> > Does this change 'from'?
>
> int Counters::value(string const & ctr) const
> {
>   CounterList::const_iterator cit = counterList.find(ctr);
>   if (cit == counterList.end()) {
>   lyxerr << "value: Counter does not exist: " << ctr << endl;
>   return 0;
>   }
>   return cit->second.value();
> }
>
> No.

But you are right to notice that the semantics of this method are very 
confusing.

1. It's a member of Counters, but does not set *this.

Is this what is meant:
void Counters::copy(Counters const & from)
{
CounterList::iterator it = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it->set(from.value(it->first));
}
}

?

2. Alternatively, it could be a non-member function

void Counters::copy(Counters const & from, Counters & to)
{
CounterList::iterator it = to.counterList.begin();
CounterList::iterator end = to.counterList.end();
for (; it != end; ++it) {
it->set(from.value(it->first));
}
}

As it is, I have no idea what it is meant to be doing, but it ain't doing it!

Angus


But is Counters::copy doing what it's meant to be doing?

Why is it a member variable that resets to

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
> 2. Alternatively, it could be a non-member function
>
> void Counters::copy(Counters const & from, Counters & to)

That's 
void copy(Counters const & from, Counters & to)

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 11:27:52AM +0200, Kornel Benko wrote:

> It loads here now, after making some scripts executable.
> (Yes, I am using the non-installed version from the src-directory.

anyone remember how we get them +x in the repository ? Do we have to
re-add them or is there a cvs thingy ?

regards
john

-- 
"It is unbecoming for young men to utter maxims."
- Aristotle

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:18 pm, John Levon wrote:
> On Fri, Aug 09, 2002 at 11:27:52AM +0200, Kornel Benko wrote:
> > It loads here now, after making some scripts executable.
> > (Yes, I am using the non-installed version from the src-directory.
>
> anyone remember how we get them +x in the repository ? Do we have to
> re-add them or is there a cvs thingy ?

either way, LyX shouldn't crash...

Re: nasty eqns lead to crash

[Lars Gullik Bjønnes]

Angus Leeming <[EMAIL PROTECTED]> writes:

| On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
>> 2. Alternatively, it could be a non-member function
>>
>> void Counters::copy(Counters const & from, Counters & to)
>
| That's 
| void copy(Counters const & from, Counters & to)

How is this then different from a member function

void operator=(Counters const & from) ??

-- 
Lgb

Re: nasty eqns lead to crash

[John Levon]

On Fri, Aug 09, 2002 at 12:51:03PM +0100, Angus Leeming wrote:

> either way, LyX shouldn't crash...

That's true. It usually involves some horrid BadDrawable, as if the
execvp failure is still trying to draw a non-existent pixmap or
something

john

-- 
"It is unbecoming for young men to utter maxims."
- Aristotle

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:30 pm, Lars Gullik Bjønnes wrote:
> Angus Leeming <[EMAIL PROTECTED]> writes:
> | On Friday 09 August 2002 12:14 pm, Angus Leeming wrote:
> >> 2. Alternatively, it could be a non-member function
> >>
> >> void Counters::copy(Counters const & from, Counters & to)
> |
> | That's
> | void copy(Counters const & from, Counters & to)
>
> How is this then different from a member function
>
> void operator=(Counters const & from) ??

Well that would copy from.counterList to this->counterList which is different 
from what I believe it is meant to be doing:

void Counters::copy(Counters const & from)
{
CounterList::iterator it  = counterList.begin();
CounterList::iterator end = counterList.end();
for (; it != end; ++it) {
it->second.set(from.value(it->first));
}
}

Anyway, patch to follow.

Angus

Re: nasty eqns lead to crash

[Angus Leeming]

On Friday 09 August 2002 1:30 pm, John Levon wrote:
> On Fri, Aug 09, 2002 at 12:51:03PM +0100, Angus Leeming wrote:
> > either way, LyX shouldn't crash...
>
> That's true. It usually involves some horrid BadDrawable, as if the
> execvp failure is still trying to draw a non-existent pixmap or
> something

can I get you to investigate since I'm sort of busy and you have been 
experiencing the problem...

A

Re: nasty eqns lead to crash

[Andre Poenitz]

On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
 ==9208== Conditional jump or move depends on uninitialised value(s)
 ==9208==at 0x40480B95: ostream::operator(int) (in 
/usr/lib/libstdc++-3-libc6.2-2-2.10.0.so)
 ==9208==by 0x8163A63: {anonymous}::initSymbols(void) (math_factory.C:154)
 ==9208==by 0x8163B04: initMath(void) (math_factory.C:167)
 ==9208==by 0x814B07A: InsetFormulaBase::InsetFormulaBase(void) 
(formulabase.C:101)

 Andre ? Martin ?

The initMath code is correct.


 clicking on a formula :
 
 ==9208== Conditional jump or move depends on uninitialised value(s)
 ==9208==at 0x817141A: MathIterator::operator++(void) (math_iterator.C:100)
 ==9208==by 0x8155088: MathCursor::bruteFind(int, int, int, int, int, int) 
(math_cursor.C:1164)
 ==9208==by 0x815244B: MathCursor::setPos(int, int) (math_cursor.C:283)
 ==9208==by 0x814B958: InsetFormulaBase::insetButtonPress(BufferView *, int, int, 
mouse_button::state) (formulabase.C:347)
 
 Andre ?

I can't see anything wrong here, either.

Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)

Re: nasty eqns lead to crash

[Andre Poenitz]

On Thu, Aug 08, 2002 at 07:15:23PM +0100, John Levon wrote:
> ==9208== Conditional jump or move depends on uninitialised value(s)
> ==9208==at 0x40480B95: ostream::operator<<(int) (in 
>/usr/lib/libstdc++-3-libc6.2-2-2.10.0.so)
> ==9208==by 0x8163A63: {anonymous}::initSymbols(void) (math_factory.C:154)
> ==9208==by 0x8163B04: initMath(void) (math_factory.C:167)
> ==9208==by 0x814B07A: InsetFormulaBase::InsetFormulaBase(void) 
>(formulabase.C:101)

> Andre ? Martin ?

The initMath code is correct.


> clicking on a formula :
> 
> ==9208== Conditional jump or move depends on uninitialised value(s)
> ==9208==at 0x817141A: MathIterator::operator++(void) (math_iterator.C:100)
> ==9208==by 0x8155088: MathCursor::bruteFind(int, int, int, int, int, int) 
>(math_cursor.C:1164)
> ==9208==by 0x815244B: MathCursor::setPos(int, int) (math_cursor.C:283)
> ==9208==by 0x814B958: InsetFormulaBase::insetButtonPress(BufferView *, int, int, 
>mouse_button::state) (formulabase.C:347)
> 
> Andre ?

I can't see anything wrong here, either.

Andre'

-- 
Those who desire to give up Freedom in order to gain Security,
will not have, nor do they deserve, either one. (T. Jefferson)