This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v2] Added file properties to windows gdb executable for all mingw32 builds.


On 08/22/2013 04:09 PM, Eli Zaretskii wrote:

> The GNU project doesn't like calling Windows a "win", so I suggest to
> rename the files and the script to use something like mingw instead.

I'd think this works just as well on Cygwin binaries?  I suggest
windows-*.

"properties" sounds odd to me, given files are usually called
resource files.  IIRC, the frequent file name used in lots of
projects is winres.rc.  But then, if you have more than one
program in your project, you'd go with gdbres.rc, foores.rc,
etc.  Which leads me to ...  We already have some support for
including resources (icon, etc.) in the binary, for gdbtk.
See gdb/configure.ac, gdb/Makefile.in and gdbtk/gdb.rc etc.
Isn't this going to conflict on gdbtk builds?

While at it, follows a review of the patch.

> +# shell parameters
...

> +
> +# default option values
> +# keep these defaults in sync with gdb/top.c print_gdb_version()

...

> +# check for environment variables to replace certain file properties

Please make the comments follow the GNU style.  That is,
Start sentences with upper case, and end them with a double period.

> +   Patch this header file using create-win_exe_properties.sh during gdb build
> +   to customize the file properties. */

Likewise, double period.  There may be more instances.

> +# keep these defaults in sync with gdb/top.c print_gdb_version()
...
> +copyright="Copyright (C) 2013 Free Software Foundation, Inc."

That "keep in sync" comment is useless, as nobody will
remember to look here when looking at top.c.  Instead,
remove that, and you'll need to update the "Start of New Year
Procedure" section in the internals manual.  Guess that means
in the wiki now, once this patch is in.

> +win_exe_properties.h: Makefile version.in common/create-win_exe_properties.sh
> +	$(SHELL) $(srcdir)/common/create-win_exe_properties.sh $(srcdir) \
> +		"$(host_alias)" "$(target_alias)" win_exe_properties.h

It's better to write to a temporary file, and then move to the final
destination, so you don't end up with a half baked file if you cancel
the build at the wrong time.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]