[PATCH 1/4] dive.h: add a 'unsafe' variant of FOR_EACH_PICTURE

Dirk Hohndel dirk at hohndel.org
Tue Oct 28 07:38:53 PDT 2014


On Tue, Oct 28, 2014 at 01:35:13PM +0200, Lubomir I. Ivanov wrote:
> From: "Lubomir I. Ivanov" <neolit123 at gmail.com>
> 
> This prevents a warning caused by -Waddress, that the address
> of 'displayed_dive' will always be defined.

There are people who have strong feelings about making code changes just
to shut up compiler warnings... :-)

> the warning has been creeping around for 5 mounths or so, but
> is overall harmless.
> i don't like this change because it creates a useless interface
> macro.

Correct.

> the other solution was cleaner:
> struct dive *displayed_dive_tmp = &displayed_dive;
> ...
> 	FOR_EACH_PICTURE_UNSAFE(displayed_dive_tmp) {
> 
> comments?

I still find that change rather silly as well.

> on the other hand global vars shouldn't be used in such a manner.
> allocating a context struct on the heap and storing everything
> supposedly global in there is one way to do it properly.

Oh I love programming philosophy discussions as much as anyone else...
Globals by themselves aren't evil. There are many usecases where they can
be abused.

Granted, the "displayed_dive" was kind of a crude tool to solve the
underlying issue I was fighting at the time, but I would argue that it is
one of the sane use cases for a global variable.

So if this warning really keeps you up at night (just kidding) I'll
propose this: create a second macro

FOR_EACH_PICTURE_NON_PTR(_divestruct) \
	for (struct picture *picture = (_divestruct).picture_list; picture; picture = picture->next)

and pass the structure to it instead of a pointer to the structure

/D


More information about the subsurface mailing list