LVCssDeclaration::parse(): use auto-resizing buffer

[poire-z]

The fixed sized buffer buf[512] can be overflowed (and cause segfaults) on biG CSS declaration (eg: containing many font names in a font-family: property).
We use SerialBuf instead and enforce adding 4-bytes ints.

Fix crash with China.EN.epub as investigated around koreader/koreader#3912 (comment)

I checked with some big test books that caches written with/without this change are readable with no problem (no styleHash mismatch, and quick loading) by crengine without/with this change, so I guess this gives the same result at the end.

I just wonder if

crengine/crengine/src/lvstring.cpp

Lines 4725 to 4734 in 42dbcf2

	SerialBuf & SerialBuf::operator << ( lUInt32 n )
	{
	if ( check(4) )
	return *this;
	_buf[_pos++] = (lUInt8)(n & 255);
	_buf[_pos++] = (lUInt8)((n>>8) & 255);
	_buf[_pos++] = (lUInt8)((n>>16) & 255);
	_buf[_pos++] = (lUInt8)((n>>24) & 255);
	return *this;
	}

and

+        // Could that cause problem with different endianess?
+        buf.copyTo( (lUInt8*)_data, buf.pos() );
+        // Alternative:
+        //   buf.setPos(0);
+        //   for (int i=0; i<sz; i++)
+        //      buf >> _data[i];

works fine by chance that we got the right endianness or not (do all our platforms share the same endianness as my emulator on x86-64?)

[Frenzie]

Would it matter at all except in the sense that the cache might be incompatible between architectures?

[Frenzie]

So even though it says store it does the whole thing every page load, huh? :-)

[poire-z]

I think it's fine for cache, as SerialBuf decides about the order the 4bytes of a lUint32 are stored in the SerialBuf, and give the lUint32 back correctly as it knows how to decode them. So, I think this code is using Little-endian.
I think buf.copyTo( (lUInt8*)_data, buf.pos() );, copying the whole thing as is, will push little-endian lUint32 into _data. If the platform is using big-endianess, the lUInt32 may be reversed and so give the wrong number.
So, do we have some big-endian arch to thest that on? :)

[poire-z]

So even though it says store it does the whole thing every page load, huh? :-)

Oh, forgot to talk about this.
It stores it I guess, but the code is explicitely asking for a full-reparsing of stylesheets.

There is this that is called before each rendering (each page turn, menu open...):

crengine/crengine/src/lvdocview.cpp

Lines 2430 to 2461 in 42dbcf2

	void LVDocView::setRenderProps(int dx, int dy) {
	if (!m_doc || m_doc->getRootNode() == NULL)
	return;
	updateLayout();
	m_showCover = !getCoverPageImage().isNull();
	if (dx == 0)
	dx = m_pageRects[0].width() - m_pageMargins.left - m_pageMargins.right;
	if (dy == 0)
	dy = m_pageRects[0].height() - m_pageMargins.top - m_pageMargins.bottom
	- getPageHeaderHeight();
	lString8 fontName = lString8(DEFAULT_FONT_NAME);
	m_font = fontMan->GetFont(m_font_size, 400 + LVRendGetFontEmbolden(),
	false, DEFAULT_FONT_FAMILY, m_defaultFontFace);
	//m_font = LVCreateFontTransform( m_font, LVFONT_TRANSFORM_EMBOLDEN );
	m_infoFont = fontMan->GetFont(m_status_font_size, 400, false,
	DEFAULT_FONT_FAMILY, m_statusFontFace);
	if (!m_font || !m_infoFont)
	return;
	updateDocStyleSheet();
	m_doc->setRenderProps(dx, dy, m_showCover, m_showCover ? dy
	+ m_pageMargins.bottom * 4 : 0, m_font, m_def_interline_space, m_props);
	text_highlight_options_t h;
	h.bookmarkHighlightMode = m_props->getIntDef(PROP_HIGHLIGHT_COMMENT_BOOKMARKS, highlight_mode_underline);
	h.selectionColor = (m_props->getColorDef(PROP_HIGHLIGHT_SELECTION_COLOR, 0xC0C0C0) & 0xFFFFFF);
	h.commentColor = (m_props->getColorDef(PROP_HIGHLIGHT_BOOKMARK_COLOR_COMMENT, 0xA08000) & 0xFFFFFF);
	h.correctionColor = (m_props->getColorDef(PROP_HIGHLIGHT_BOOKMARK_COLOR_CORRECTION, 0xA00000) & 0xFFFFFF);
	m_doc->setHightlightOptions(h);
	}

It calls among other things:

crengine/crengine/src/lvdocview.cpp

Lines 491 to 494 in 42dbcf2

	void LVDocView::updateDocStyleSheet() {
	CRPropRef p = m_props->getSubProps("styles.");
	m_doc->setStyleSheet(substituteCssMacros(m_stylesheet, p).c_str(), true);
	}

I believe we could get rid of this, as it looks to me it's for CoolReader behaviour: with CoolReader, we may change from the UI various settings, including styles, so that's crengine way of knowing some things have changed and that the rendering needs to take them into account.
With KOReader, as for each typeset/layout change, we are explicitely calling methods on crengine to set them, before rendering, we may not need that section.

Timing that updateDocStyleSheet says 3ms with a big book on the emulator, so like 30ms on a device.
So, not essential to remove, but I'd like to do it anyway, later when I've got no more crengine changes in the pipe I guess :)

[Frenzie]

So, do we have some big-endian arch to thest that on? :)

I think some ARMs at least support big endian. I don't have anything other than the generic ARMv7 HF in the house though, both on Android and Kobo.

[poire-z]

So, should that go into tonight's crengine/base bump?

[Frenzie]

Fine by me. :-)