New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patches to fix various -W -Wall warnings, enabling -W -Wall flags, .map and .lst files, and more verbose size report #14

Closed
wants to merge 12 commits into
base: master
from

Conversation

4 participants
@jcwren

jcwren commented Dec 21, 2010

8 of these patches modify various files that produce warnings when the -W -Wall flags are enabled.

The Compiler.java patch adds the preferences.txt options to enable -W -Wall flags to the compiler, producing a .map file, producing a .lst file, and enabling more verbose size reports of the program and data spaces.

jcwren added some commits Dec 21, 2010

Compiler.java: adds options to support -W,-Wall flags, .map file and …
….lst file generation, avr-size output

This patch adds 4 preferences.txt options to support enabling -W and -Wall
flags when compiling, producing a .map file as part of the linking stage,
producing a diassembly list file after linking, and a more verbose size output.

Adding 'compiler.show_all_warnings=true' to preferences.txt adds the -W and
-Wall flags passed to the compiler, and removes the -w flag.  This produces
informative verbose warnings about less than ideally constructed code, such as
signed vs unsigned comparisons, unused variables, and a great deal of other
useful information.

Adding 'compiler.map_file=true' causes the linker to produce a detailed map
file listing of the code.  This file is written to the directory the sketch is
located in.

Adding 'compiler.lst_file=true' causes avr-objdump to be run on the produced
.elf file.  The listing file contains a disassembled listing of the code
intermixed with the source code.  The -d (Display assembler contents of
executable sections) and -S (Intermix source code with disassembly) options are
passed to avr-objdump.  The resulting .lst file is written to the directory the
sketch is located in.

Adding 'compiler.detailed_size' causes avr-size to be run on the produced .elf
file.   This displays a more detailed report the combined sizes of the program
space (.text + .data + .bootloader), and the combined sizes of data space
(.data + .bss + .noinit)

Unfortunately, avr-objdump does not support a command line option to specify
where to write the output of the program to.  Instead, EVERYTHING goes to
stdout.  Normally anything written to stdout by an exec'ed program is written
to the status window of the Arduino IDE.  To get the the stdout of avr-objdump
captured to be written to an output file required a tad bit hackishness in the
execAsynchronously function.  See source for details.
Print: fix signed vs unsigned comparison
Print currently uses a signed vs unsigned comparison in Print::print(const String &s) while iterating through individual characters.
This causes compiler warnings when using -W -Wall.
WInterrupts.c: warning: 'static' is not at beginning of declaration
The static keyword is improperly placed in the declaration of the intFunc
variable.
WString.cpp: 'base' variable is unused, but should be passed to ultoa()
Compiling with -W -Wall flags revealed that the 'base' variable in the
parameter list for String::String( const long value, const int base )
was not being passed to the ultoa() function.  Instead, the parameter
to ultoa() was hard-coded to 10.
WString.h: fix type qualifiers ignored warning
The length() function has a useless 'const' keyword in the declation, causing
the compiler to throw a "type qualifiers ignored on function return type"
warning when compiled with -W -Wall.
pins_arduino.c: integer from pointer cast warning
This patch fixes the "initialization makes integer from pointer without a cast"
warning generated by the compiler when using the -W -Wall flags.  The data type
in the array is a uint16_t, but DDRA (for example) is a pointer.  A cast is
required to make GCC happy.
pins_arduino.h: fix compiler warning
The SPI pins are declared as 'const static' instead of 'static const', causing
the compiler to throw a warning with the -W -Wall flags are used.
wiring_private.h: fix compiler warning
When compiling with the -W -Wall flags, a warning is emitted indicating that
<avr/delay.h> has been moved to <util/delay.h>.  This patch fixes the #include
to point to the new location.
Sd2Card.cpp: fix compiler warning
All the while() loops that check for the SPI transfer to be complete have the
semi-colon immediately after the closing parenthesis.  This both causes a
compiler warning of "warning: suggest a space before ';' or explicit braces
around empty body in 'while' statement", and is considered a less-than-ideal
programming practice.  This patch breaks the semi-colon on to the next line,
both eliminating the compiler error and making the code more readable.

In all probability the test should be moved into a macro or a inlineable
sub-routine.
SD.c: Fix error in comment for remove()
Comment was duplicated from mkdir() and not updated.
HardwareSerial.cpp: fix comment in flush()
The comments for flush() had duplicated lines of text (probably a cut and paste
error)
@matthijskooijman

This comment has been minimized.

Show comment
Hide comment
@matthijskooijman

matthijskooijman Feb 18, 2014

Collaborator

I just went over these patches. Most of them have already been applied, or the relevent code has been removed. Two patches to the SD library were still relevant, I included those in the pullrequest I just sent. The first patch to Compiler.java contains a part that adds an option to enable warnings in the build, which might still be relevant, though I doubt that it still applies to the current code.

I think this pullrequest can be closed.

Collaborator

matthijskooijman commented Feb 18, 2014

I just went over these patches. Most of them have already been applied, or the relevent code has been removed. Two patches to the SD library were still relevant, I included those in the pullrequest I just sent. The first patch to Compiler.java contains a part that adds an option to enable warnings in the build, which might still be relevant, though I doubt that it still applies to the current code.

I think this pullrequest can be closed.

@ffissore ffissore added the New label Feb 27, 2014

@cmaglie cmaglie added 1.0 and removed New labels Apr 13, 2014

@cmaglie

This comment has been minimized.

Show comment
Hide comment
@cmaglie

cmaglie Apr 20, 2014

Member

Merged with #1877.

Member

cmaglie commented Apr 20, 2014

Merged with #1877.

@cmaglie cmaglie closed this Apr 20, 2014

tbowmo pushed a commit to tbowmo/Arduino that referenced this pull request Jul 14, 2016

Merge pull request arduino#14 from Timminater/patch-1
Update misspelled comment in Config.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment