Re: [PATCH][stage1] Remove conditionals around free()
On 3/4/23, Janne Blomqvist via Gcc-patches wrote: > On Wed, Mar 1, 2023 at 11:31 PM Bernhard Reutner-Fischer via Fortran > wrote: >> >> Hi! >> >> Mere cosmetics. >> >> - if (foo != NULL) >> free (foo); >> >> With the caveat that coccinelle ruins replacement whitespace or i'm >> uneducated enough to be unable to _not_ run the diff through >> sed -e 's/^+\([[:space:]]*\)free(/+\1free (/' >> at least. If anybody knows how to improve replacement whitespace, >> i'd be interrested but didn't look nor ask. ISTM that leading >> whitespace is somewhat ruined, too, so beware (8 spaces versus tab as >> far as i have spot-checked). >> >> Would touch >> gcc/ada/rtinit.c |3 +-- >> intl/bindtextdom.c |3 +-- >> intl/loadmsgcat.c|6 ++ >> intl/localcharset.c |3 +-- >> libbacktrace/xztest.c|9 +++-- >> libbacktrace/zstdtest.c |9 +++-- >> libbacktrace/ztest.c |9 +++-- >> libgfortran/caf/single.c |6 ++ >> libgfortran/io/async.c |6 ++ >> libgfortran/io/format.c |3 +-- >> libgfortran/io/transfer.c|6 ++ >> libgfortran/io/unix.c|3 +-- >> libgo/runtime/go-setenv.c|6 ++ >> libgo/runtime/go-unsetenv.c |3 +-- >> libgomp/target.c |3 +-- >> libiberty/concat.c |3 +-- >> zlib/contrib/minizip/unzip.c |2 +- >> zlib/contrib/minizip/zip.c |2 +- >> zlib/examples/enough.c |6 ++ >> zlib/examples/gun.c |2 +- >> zlib/examples/gzjoin.c |3 +-- >> zlib/examples/gzlog.c|6 ++ >> >> coccinelle script and invocation inline. >> Would need to be split for the respective maintainers and run through >> mklog with subject changelog and should of course be compiled and >> tested before that. >> >> Remarks: >> 1) We should do this in if-conversion (?) on our own. >>I suppose. Independently of -fdelete-null-pointer-checks >> 2) Maybe not silently, but raise language awareness nowadays. >>By now it's been a long time since this was first mandated. >> 3) fallout from looking at something completely different >> 4) i most likely will not remember to split it apart and send proper >>patches, tested patches, in stage 1 to maintainers proper, so if >>anyone feels like pursuing this, be my guest. I thought i'd just >>mention it. >> >> cheers, > > Back in 2011 Jim Meyering applied a patch doing this, see > https://gcc.gnu.org/legacy-ml/gcc-patches/2011-03/msg00403.html , and > the gcc/README.Portability snippet added then which is still there. > > Per analysis done then, it seems SunOS 4 was the last system where > free() of a NULL pointer didn't behave per the spec. > > Also in Jim's patch intl/ and zlib/ directories were not touched as > those are imported from other upstreams. > Speaking of which, this reminded me that I opened bug 80528 to catch code like this as a compiler warning: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80528 > -- > Janne Blomqvist >
Re: [PATCH][WIP] Add install-dvi Makefile targets
On Fri, Oct 22, 2021 at 8:23 AM Jeff Law wrote: > > > > On 10/18/2021 7:30 PM, Eric Gallager wrote: > > On Tue, Oct 12, 2021 at 5:09 PM Eric Gallager wrote: > >> On Thu, Oct 6, 2016 at 10:41 AM Eric Gallager wrote: > >>> Currently the build machinery handles install-pdf and install-html > >>> targets, but no install-dvi target. This patch is a step towards > >>> fixing that. Note that I have only tested with > >>> --enable-languages=c,c++,lto,objc,obj-c++. Thus, target hooks will > >>> probably also have to be added for the languages I skipped. > >>> Also, please note that this patch applies on top of: > >>> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00370.html > >>> > >>> ChangeLog: > >>> > >>> 2016-10-06 Eric Gallager > >>> > >>> * Makefile.def: Handle install-dvi target. > >>> * Makefile.tpl: Likewise. > >>> * Makefile.in: Regenerate. > >>> > >>> gcc/ChangeLog: > >>> > >>> 2016-10-06 Eric Gallager > >>> > >>> * Makefile.in: Handle dvidir and install-dvi target. > >>> * ./[c|cp|lto|objc|objcp]/Make-lang.in: Add dummy install-dvi > >>> target hooks. > >>> * configure.ac: Handle install-dvi target. > >>> * configure: Regenerate. > >>> > >>> libiberty/ChangeLog: > >>> > >>> 2016-10-06 Eric Gallager > >>> > >>> * Makefile.in: Handle dvidir and install-dvi target. > >>> * functions.texi: Regenerate. > >> Ping. The prerequisite patch that I linked to previously has gone in now. > >> I'm not sure if this specific patch still applies, though. > >> Also note that I've opened a bug to track this issue: > >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102663 > > Hi, I have updated this patch and tested it with more languages now; I > > can now confirm that it works with ada, d, and fortran now. The only > > languages that remain untested now are go (since I'm building on > > darwin and go doesn't build on darwin anyways, as per bug 46986) and > > jit (which I ran into a bug about that I brought up on IRC, and will > > probably need to file on bugzilla). OK to install? > Yea, I think this is OK. We might need to adjust go/jit and perhaps > other toplevel modules, but if those do show up as problems I think we > can fault in those fixes. > > jeff OK thanks, installed as r12-4636: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=c3e80a16af287e804b87b8015307085399755cd4
Re: [PATCH][WIP] Add install-dvi Makefile targets
On Tue, Oct 19, 2021 at 1:41 AM Thomas Koenig wrote: > > Hi Eric, > > > Hi, I have updated this patch and tested it with more languages now; I > > can now confirm that it works with ada, d, and fortran now. The only > > languages that remain untested now are go (since I'm building on > > darwin and go doesn't build on darwin anyways, as per bug 46986) and > > jit (which I ran into a bug about that I brought up on IRC, and will > > probably need to file on bugzilla). OK to install? > > Fortran parts look good. > > Best regards > > Thomas OK, thanks... so... at this point, who do I still need approval from for the rest of it, then? Should I be cc-ing the build system maintainers? The maintainers for all the rest of the subdirectories I'm touching? Global reviewers? Someone else? Thanks, Eric Gallager
Re: [PATCH][WIP] Add install-dvi Makefile targets
On Tue, Oct 12, 2021 at 5:09 PM Eric Gallager wrote: > > On Thu, Oct 6, 2016 at 10:41 AM Eric Gallager wrote: > > > > Currently the build machinery handles install-pdf and install-html > > targets, but no install-dvi target. This patch is a step towards > > fixing that. Note that I have only tested with > > --enable-languages=c,c++,lto,objc,obj-c++. Thus, target hooks will > > probably also have to be added for the languages I skipped. > > Also, please note that this patch applies on top of: > > https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00370.html > > > > ChangeLog: > > > > 2016-10-06 Eric Gallager > > > > * Makefile.def: Handle install-dvi target. > > * Makefile.tpl: Likewise. > > * Makefile.in: Regenerate. > > > > gcc/ChangeLog: > > > > 2016-10-06 Eric Gallager > > > > * Makefile.in: Handle dvidir and install-dvi target. > > * ./[c|cp|lto|objc|objcp]/Make-lang.in: Add dummy install-dvi > > target hooks. > > * configure.ac: Handle install-dvi target. > > * configure: Regenerate. > > > > libiberty/ChangeLog: > > > > 2016-10-06 Eric Gallager > > > > * Makefile.in: Handle dvidir and install-dvi target. > > * functions.texi: Regenerate. > > Ping. The prerequisite patch that I linked to previously has gone in now. > I'm not sure if this specific patch still applies, though. > Also note that I've opened a bug to track this issue: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102663 Hi, I have updated this patch and tested it with more languages now; I can now confirm that it works with ada, d, and fortran now. The only languages that remain untested now are go (since I'm building on darwin and go doesn't build on darwin anyways, as per bug 46986) and jit (which I ran into a bug about that I brought up on IRC, and will probably need to file on bugzilla). OK to install? patch-install-dvi.diff Description: Binary data
Re: [PATCH] libgfortran : Use the libtool macro to determine libm availability.
On Fri, Aug 20, 2021 at 3:53 AM Tobias Burnus wrote: > > On 20.08.21 09:34, Richard Biener via Fortran wrote: > > > On Thu, Aug 19, 2021 at 10:10 PM Iain Sandoe wrote: > >> libm is not needed on Darwin, and should not be added unconditionally > >> even if that is (mostly) harmless since it is a symlink to libc. > >> > >> tested on x86_64, i686-darwin, x86_64-linux, > >> OK for master? > > OK. > > Richard. > > Looks also good to me – but for completeness and as background, I also > want to remark the following. > > (At least as I understand it, I did not dig deeper.) > > > --- a/libgfortran/configure.ac > > +++ b/libgfortran/configure.ac > > ... > > +AC_CHECK_LIBM > > This CHECK_LIBM has a hard-coded list of targets and for some (like > Darwin) it simply does not set $LIBM. I thought the proper macro to use was LT_LIB_M ? > > >> +++ b/libgfortran/Makefile.am > >> @@ -42,7 +42,7 @@ libgfortran_la_LINK = $(LINK) $(libgfortran_la_LDFLAGS) > >> libgfortran_la_LDFLAGS = -version-info `grep -v '^\#' > >> $(srcdir)/libtool-version` \ > >> $(LTLDFLAGS) $(LIBQUADLIB) ../libbacktrace/libbacktrace.la \ > >> $(HWCAP_LDFLAGS) \ > >> - -lm $(extra_ldflags_libgfortran) \ > >> + $(LIBM) $(extra_ldflags_libgfortran) \ > > I think usage like this one do not actually link '-lm' as > gcc/config/darwin.h contains: > > #define LINK_SPEC \ > ... > %:remove-outfile(-lm) \ > > However, this spec change comes too early for: > > --- a/libgfortran/libgfortran.spec.in > > +++ b/libgfortran/libgfortran.spec.in > > @@ -5,4 +5,4 @@ > > # > > > > %rename lib liborig > > -*lib: @LIBQUADSPEC@ -lm %(libgcc) %(liborig) > > +*lib: @LIBQUADSPEC@ @LIBM@ %(libgcc) %(liborig) > NIT: There are two spaces instead of one before @LIBM@. > > Thus, it makes sense to the unconditional '-lm' from the .spec file. > > Tobias > > - > Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 > München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas > Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht > München, HRB 106955
