[riot-devel] Byte array should be uint8_t, not char

Kees Bakker kees at sodaq.com
Fri Sep 30 20:45:54 CEST 2016


Hi Joakim,

I've done a PR for SPI. https://github.com/RIOT-OS/RIOT/pull/5901
If you have a chance, can you have a look at it/

What this set of changes shows is that several developers have been
inserting casts to be able to get SPI code compile without errors. These 
casts
(from uint8_t to char) are now not needed anymore.

Yes, there were quite a few files to be modified, but the changes were 
straightforward.
I even patched a patch file :-) See 
pkg/u8g2/patches/0002-u8g2-add-riot-os-interface.patch
-- Kees

On 17-07-16 18:40, Joakim Nohlgård wrote:
> Hi Kees,
> Do you have the time/energy to resume the effort of updating the 
> periph/spi interface in where Hauke left off in 
> https://github.com/RIOT-OS/RIOT/pull/4780?
>
> There are a lot of device drivers which need updating, and also some 
> existing comments on the PR.
>
> I can assist in reviewing the changes and updating device drivers, but 
> I don't have a lot of time to spend on this right now.
>
> Best regards,
> Joakim
>
> On Wed, Jul 6, 2016 at 7:00 AM, Ludwig Knüpfer 
> <ludwig.knuepfer at fu-berlin.de <mailto:ludwig.knuepfer at fu-berlin.de>> 
> wrote:
>
>     Hi Kees,
>
>     I assume there are more violations than the ones in the SPI
>     drivers... But of course every step forward is great!
>
>     Testing can be done by the community. We have plans to create a
>     distributed test system with actual hardware attached, but sadly
>     that has not become reality yet.
>
>     Compilation can be tested automatically by running `make
>     buildtest` for the respective test application (tests/periph_spi).
>
>     Cheers,
>     Ludwig
>
>     Am 5. Juli 2016 21:31:49 MESZ, schrieb Kees Bakker <kees at sodaq.com
>     <mailto:kees at sodaq.com>>:
>     >Hi Ludwig,
>     >
>     >Well, it will be a challenge to smootly correct this.
>     >There are 16 CPU's that use spi_transfer_byte(s) and 6 drivers.
>     >
>     >I won't mind creating a PR, but of course I can only test it by
>     >building
>     >examples for all boards that support SPI. And look at compile errors.
>     >Or are
>     >there other procedures?
>     >
>     >On 04-07-16 07:23, Ludwig Knüpfer wrote:
>     >> Hi Kees,
>     >>
>     >> Unless there is a good reason to deviate from this guideline all
>     >violations should be corrected. This particular rule was added
>     >relatively recently, so it would not surprise me if not all
>     occurrences
>     >in RIOT have been adapted yet.
>     >>
>     >> Cheers,
>     >> Ludwig
>     >>
>     >> Am 3. Juli 2016 22:50:10 MESZ, schrieb Kees Bakker
>     <kees at sodaq.com <mailto:kees at sodaq.com>>:
>     >>> Hi,
>     >>>
>     >>> The Coding Convention is clear about it.
>     >>>
>     >>> "Guidelines for pointer types (as long as it is reasonable):
>     >>>
>     >>>   * use |char *| for strings and only for strings
>     >>> * use |uint8_t[]| as type for arbitrary byte buffers, but use
>     |void
>     >*|
>     >>>     to pass them around. |uint8_t[]| because we're dealing with
>     >bytes
>     >>>     and not characters, |void *| to avoid unnecessary casting
>     shall
>     >the
>     >>>     need arise to have their content to have a certain type
>     >>>   * use |uint8_t *| to pass "typed" byte buffers, e.g., link-layer
>     >>>     addresses, where it avoids unnecessary temporary variable
>     >>>   * use |void *| for generic typing"
>     >>>
>     >>>
>     >>> In the SPI driver however the transfer functions use char *
>     >parameters,
>     >>>
>     >>> but SPI is usually dealing with binary
>     >>> information (bytes), not strings. This leads to unnecessary
>     casts in
>     >>> other parts of the code. (E.g. nvram_spi).
>     >>>
>     >>> What is our policy about this? Are we going to correct this at
>     some
>     >>> point? Is it too late already (I hope not)?
>     >> _______________________________________________
>     >> devel mailing list
>     >> devel at riot-os.org <mailto:devel at riot-os.org>
>     >> https://lists.riot-os.org/mailman/listinfo/devel
>     >
>     >
>     >--
>     >Kees Bakker
>     >Founder
>     >SODAQ
>     >M. 0031617737165
>     >www.sodaq.com <http://www.sodaq.com>
>     >
>     >_______________________________________________
>     >devel mailing list
>     >devel at riot-os.org <mailto:devel at riot-os.org>
>     >https://lists.riot-os.org/mailman/listinfo/devel
>
>     _______________________________________________
>     devel mailing list
>     devel at riot-os.org <mailto:devel at riot-os.org>
>     https://lists.riot-os.org/mailman/listinfo/devel
>
>
>
>
> _______________________________________________
> devel mailing list
> devel at riot-os.org
> https://lists.riot-os.org/mailman/listinfo/devel


-- 
Kees Bakker
Founder
SODAQ
M. 0031617737165
www.sodaq.com

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/devel/attachments/20160930/4d72ae9c/attachment-0001.html>


More information about the devel mailing list