Bug 99660 - unreachable condition in generated code OOXMLFactory_shared-math.cxx
Summary: unreachable condition in generated code OOXMLFactory_shared-math.cxx
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: filters and storage (show other bugs)
Version:
(earliest affected)
3.3.1 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL: http://www.datypic.com/sc/ooxml/t-m_S...
Whiteboard:
Keywords: filter:ooxml
Depends on:
Blocks: Dev-related
  Show dependency tree / graph
 
Reported: 2016-05-03 18:12 UTC by dcb314
Modified: 2023-05-13 14:26 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description dcb314 2016-05-03 18:12:28 UTC 
1.

core/workdir/CustomTarget/writerfilter/source/ooxml/OOXMLFactory_shared-math.cxx:461]: (style) Expression is always false because 'else if' condition matches previous condition at line 460.

Source code is

            if (rValue == "left") { rOutValue = NS_ooxml::LN_Value_math_ST_Jc_start; }
            else if (rValue == "left") { rOutValue = NS_ooxml::LN_Value_math_ST_Jc_left; }

2.

Similar thing a few lines further down

            if (rValue == "right") { rOutValue = NS_ooxml::LN_Value_math_ST_Jc_end; }
            else if (rValue == "right") { rOutValue = NS_ooxml::LN_Value_math_ST_Jc_right; }
Comment 1 JoNi 2016-05-03 19:52:49 UTC 
thank you for your report

OOXMLFactory_shared-math.cxx is generated from writerfilter/source/ooxml/model.xml
the lines 8218 and 8219 are responsible for the findings.

introduced by commit b17f4ecea142fd6f311dcb457e8896cd12e60fc0 in LO 4.4 branch and all following

hmm, start means align to leading edge
and end means align to tailing edge
I don't see the difference to left and right aligned. so maybe it's correct and was just forgotten in the commit message?
Comment 2 JoNi 2016-05-03 22:04:58 UTC 
as Micheal Stahl noted it was actually introduced in commit 794c8cf13bd0c8512bf77c57934abd9b9a0fe998
[Docx] n#513479: import start/end values for ST_Jc
Comment 3 Julien Nabet 2016-05-04 07:11:03 UTC 
So we should just replace this:
<value tokenid="ooxml:Value_math_ST_Jc_start">left</value>
by this:
<value tokenid="ooxml:Value_math_ST_Jc_start">start</value>
(see http://opengrok.libreoffice.org/xref/core/writerfilter/source/ooxml/model.xml#8218)

Miklos: does it seem ok or would it break something?
Comment 4 Xisco Faulí 2016-09-19 15:29:45 UTC Comment hidden (obsolete)
Comment 5 Xisco Faulí 2017-09-29 08:52:54 UTC Comment hidden (obsolete)
Comment 6 QA Administrators 2019-12-03 15:01:32 UTC Comment hidden (obsolete)
Comment 7 QA Administrators 2021-12-03 04:46:44 UTC Comment hidden (obsolete)
Comment 8 Roman Kuznetsov 2023-05-11 12:24:31 UTC 
Julien, is this one still actual?
Comment 9 Julien Nabet 2023-05-13 14:26:50 UTC 
(In reply to Roman Kuznetsov from comment #8)
> Julien, is this one still actual?

There have been several commits on writerfilter/source/ooxml/model.xml, at least what dcb314 described doesn't appear anymore in workdir/CustomTarget/writerfilter/source/ooxml/OOXMLFactory_shared-math.cxx
Here's what we see now:
    257     case NN_shared_math|DEFINE_ST_XAlign:
    258         if (aValue.empty())
    259             return false;
    260         switch (aValue[0])
    261         {
    262         case 'c':
    263             if (aValue == "center") { rOutValue = NS_ooxml::LN_Value_math_ST_XAlign_center; }
    264             else { return false; }
    265             return true;
    266         case 'l':
    267             if (aValue == "left") { rOutValue = NS_ooxml::LN_Value_math_ST_XAlign_left; }
    268             else { return false; }
    269             return true;
    270         case 'r':
    271             if (aValue == "right") { rOutValue = NS_ooxml::LN_Value_math_ST_XAlign_right; }
    272             else { return false; }
    273             return true;
    274         }
    275         return false;

So let's put this one to WFM then.

dcb314: don't hesitate to reopen if I missed something.

Roman: thank you for the ping! :-)