[PATCH] D89120: [lib/ObjectYAML] - Simplify the code that handles Content/Size fields.

[George Rimar via Phabricator via llvm-commits]

grimar marked an inline comment as done.
grimar added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:1303-1304
     ContiguousBlobAccumulator &CBA) {
-  if (Section.Content || Section.Size) {
-    SHeader.sh_size = writeContent(CBA, Section.Content, Section.Size);
+  if (!Section.Entries)
     return;
 
----------------
jhenderson wrote:
> I can see why this is needed, but why wasn't it needed before, and what's the impact of adding it? Same applies in a few other places too.
Previously when we had either `Content` or `Size`, the code on the left wrote some data and returned.
And we report an error when both `Entries` and `Content`/`Size` are used together much earlier, it is a normal situation.

After this patch we handle `Content/Size` for all types of sections in a single common place before `writeSectionContent` method is called.
But we still have to call `writeSectionContent` to set section fields (e.g sh_entsize/sh_link etc) and/or
handle section specific `Entries` (like here). Without this condition we just crash when trying to access an empty `Entries` in a loop below.

Perhaps worth to rename `writeSectionContent` to `finalizeSection` or alike.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89120/new/

https://reviews.llvm.org/D89120