Обсуждение: Crash with empty dictionary

I'm fooling around with tsearch, and bumped into a segfault, when using
a custom ispell dictionary with DictFile pointing to an empty file.
NISortDictionary assumes there's at least one word in the dictionary,
and crashes on line 941:

>     Conf->AffixData[1] = pstrdup(Conf->Spell[0]->p.flag);

I'm not sure if this is a sane way to set up a dictionary, but surely
seg faulting is not the right thing to do. Should we throw an error on
an empty dict file, or should we swallow it without crashing?

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas <heikki@enterprisedb.com> writes:
> I'm not sure if this is a sane way to set up a dictionary, but surely
> seg faulting is not the right thing to do. Should we throw an error on
> an empty dict file, or should we swallow it without crashing?

Offhand I'd say that an empty file is a legitimate corner case,
so we should just take it silently.
        regards, tom lane

Tom Lane wrote: <blockquote cite="mid2336.1187760320@sss.pgh.pa.us" type="cite"><pre wrap="">Heikki Linnakangas <a
class="moz-txt-link-rfc2396E"href="mailto:heikki@enterprisedb.com"><heikki@enterprisedb.com></a> writes:
</pre><blockquotetype="cite"><pre wrap="">I'm not sure if this is a sane way to set up a dictionary, but surely
 
seg faulting is not the right thing to do. Should we throw an error on
an empty dict file, or should we swallow it without crashing?   </pre></blockquote><pre wrap="">
Offhand I'd say that an empty file is a legitimate corner case,
so we should just take it silently.
        regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at
               <a class="moz-txt-link-freetext"
href="http://www.postgresql.org/about/donate">http://www.postgresql.org/about/donate</a>

 </pre></blockquote><br /> Shouldn't we be warning about an empty file rather than just swallowing up the error? It
mightnot be intentional and rather than the user trying to figure it out, we should at least be informing him/her...<br
/><br/> --<br /> Hamid<br /> 

"Hamid Quddus Akhtar" <hamid.akhtar@enterprisedb.com> writes:
>> Offhand I'd say that an empty file is a legitimate corner case,
>> so we should just take it silently.

> Shouldn't we be warning about an empty file rather than just swallowing 
> up the error?

You are jumping to a conclusion, namely that it is an error.  If it's
a legitimate corner case, throwing a warning every time the file is
read would be incredibly annoying.

If it's not a legitimate case, then we should throw a real error.
A warning just strikes me as the worst of both worlds.
        regards, tom lane

Tom Lane wrote:
> "Hamid Quddus Akhtar" <hamid.akhtar@enterprisedb.com> writes:
> >> Offhand I'd say that an empty file is a legitimate corner case,
> >> so we should just take it silently.
> 
> > Shouldn't we be warning about an empty file rather than just swallowing 
> > up the error?
> 
> You are jumping to a conclusion, namely that it is an error.  If it's
> a legitimate corner case, throwing a warning every time the file is
> read would be incredibly annoying.
> 
> If it's not a legitimate case, then we should throw a real error.
> A warning just strikes me as the worst of both worlds.

A zero-length file seems fine to me in this case.

--  Bruce Momjian  <bruce@momjian.us>          http://momjian.us EnterpriseDB
http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Tom Lane wrote:
>> "Hamid Quddus Akhtar" <hamid.akhtar@enterprisedb.com> writes:
>>>> Offhand I'd say that an empty file is a legitimate corner case,
>>>> so we should just take it silently.
>>> Shouldn't we be warning about an empty file rather than just swallowing 
>>> up the error?
>> You are jumping to a conclusion, namely that it is an error.  If it's
>> a legitimate corner case, throwing a warning every time the file is
>> read would be incredibly annoying.
>>
>> If it's not a legitimate case, then we should throw a real error.
>> A warning just strikes me as the worst of both worlds.
> 
> A zero-length file seems fine to me in this case.

It also seems to have problems with an affix-file with a single entry.

Looking closer at the tmpCtx hack, it looks like it can't just be
replaced by setting CurrentMemoryContext to a temporary context. Some
stuff needs to be allocated in the ts cache entry's dictCtx, while other
stuff is temporary. I'll try to at least comment it.

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com