On Thu, 2025-10-23 at 14:16 -0400, David Malcolm wrote: > On Tue, 2025-10-21 at 16:29 -0400, David Malcolm wrote: > > On Tue, 2025-10-21 at 13:57 -0500, Robert Dubner wrote: > > > Ahoy the reviewers! > > > > Hi Bob! > > > > > > > > This is an "Is this okay for trunk?" request. > > > > > > Although these changes affect code only in libcobol and > > > gcc/cobol, > > > we > > > are > > > adding some files and some new directories, and so there are > > > changes > > > to > > > gcc/cobol/Make-lang.in and to libgcobol/Makefile.am. > > > > > > So, we figured that this time we'd ask for permission ahead of > > > time, > > > instead of our usual practice, lately, of asking for forgiveness > > > afterward. > > > > > > I applied this patch to today's gcc/master. I did a complete > > > --enable-languages=cobol bootstrap/multilib build on an Ubuntu > > > x86_64-pc-linux-gnu system. "make check-cobol" worked after > > > that. > > > > Looking at https://gcc.gnu.org/git/?p=gcc.git;a=shortlog I don't > > see > > the patch, so am I right in thinking that by "today's gcc/master", > > you're referring to the symas git repo? (sorry, I don't have the > > URL > > handy) > > > > You didn't mention this in the cover letter, but skimming through > > the > > patch, I see this adds a dependency on libxml2 to libgcobol to e.g. > > parse the XML. > > > > Whenever a patch adds add a new dependency, the patch should also > > add > > a > > mention of that dependency to gcc/doc/install.texi (presumably here > > to > > the "GCOBOL-prerequisite" item). > > > > XML parsing is an area that tends to attract security vulnerability > > reports (e.g. the "billion laughs" attack), and libxml2 is no > > exception > > to this. See e.g. https://lwn.net/Articles/1025971/ for an account > > of > > the current status of upstream and its security posture. > > > > I think the patch needs to be clear about whether the dependency on > > libxml2 is optional, and whether libgcobol's linkage against > > libxml2 > > is > > static or dynamic. I expect no shortage of future libxml2 CVEs, > > and > > people will want to know if they have to rebuild libgcobol to get > > the > > fix (static linkage), or if it happens automagically (dynamic > > linkage). > > I don't think gcc would want to add a statically-linked dependency > > on > > libxml2, given the many CVEs that project has had. > > > > > > > > So, as far as I know, it's safe for me to apply this to > > > gcc/master. > > > > > > Okay for trunk? > > Hi Bob > > I came away with the impression that there were (small but) concrete > objections to the patch as-posted [1] and thus that it would be > revised > before reaching "trunk", but... > > > > > That's not for me to say, but hopefully the above is > > helpful/constructive. > > ...I see looking here that I didn't word it that way; sorry for not > being clearer. > > Looking at https://gcc.gnu.org/cgit/gcc/log/ I see that the patch > *is* > now in gcc trunk (currently 4th from the top): > https://gcc.gnu.org/cgit/gcc/commit/?id=b20c6458fa0a9e78253052f0493e921f75641828 > as commit r16-4582-gb20c6458fa0a9e78253052f0493e921f75641828, and as > far as I can tell, it's in the form you posted to the list without > the > various nitpicks fixed. > > Did you mean to push it into this repo's trunk, or was this a misfire > in git? (git has a terrible UI...) > > Is the plan to address the problems with the patch as followups on > trunk (I can file bug reports if that would help), or to revert that > commit on trunk and to redo the work on your own repo? Either way > probably works, and the former is probably easier, but right now the > "review" on this patch is in a weird in-the-middle kind of state. > The > way I see our process is that if someone points something out in > patch > review, it should be addressed before the patch reaches trunk (or the > reviewer explicitly says that a particular nitpick can be left to be > cleaned up in followup work). > > Chatting with Jim earlier it sounds like XML support is a necessary > feature for a cobol implementation to be credible, so the libxml2 > dependency should probably be mandatory if cobol is enabled at > configure time, but optional for people building gcc without the > cobol > frontend (if that makes sense). > > Sorry for not being clearer about my objections/nit-picks; and I hope > this doesn't come across too much like dunking on the new(ish) guy > (sorry if that's the case) > > Dave > > [1] IIRC, I wanted more explicit acknowledgement of the new libxml2 > dependency and its scope, and there were some stray files that Richi > spotted
...and I see you've addressed almost all of this in your other email, which I didn't see before sending the above one; sorry. Dave
