Some design suggestions

Karsten Behrmann BearPerson at gmx.net
Mon Jun 28 11:32:20 UTC 2010


As Bruce Cran wrote:
> Quoting Karsten Behrmann:
>> I suggest having each typed as
>> int configFoo(void)
>> where appropriate, with the following return codes:
>> * 0 - success
>> * 1 - unspecified error (to catch returns of a && b)
>> * 2 - user initiated abort (by pressing cancel or something)
>> * 3 - operation failed (no disk found, I/O error, ...)
> 
> I'd prefer something like:
> 
> struct foo* initFoo(void);
> 
> Then, pass the foo* around each of the functions. I think
> that having fewer globals is a good thing. Also, we should typedef the
> return type to make it clear what it refers to.

I was thinking we might do an enum for the status, but that might
make callers overly verbose, not sure...
"int rc" just is shorter than "enum SysinstallReturn rc"

[see below for more ranting on "fewer globals" - suffice to say that I
strongly disagree with the foo* .]

> I agree - we should be aiming towards a UI-agnostic API so that in the
> future a text or GUI frontend can be plugged in if pc-sysinstall ends
> up not taking over.

This is a side effect, but not what I'm driving at (yet). Having UI-
agnostic API helps writing different frontends, but since the backend
functions heavily interact with the user, we would need a UI-agnostic
UI library anyway before we can really plug in different UIs.

Instead, my concern is that the main parts of the API should not be
burdened by the thought "how should I influence the menu when I am
used inside one" - regardless of which types and conventions we use
to reflect that. There is more than enough these functions need to
worry about, already.

>> ----- use more static data -----
>> [TL;DR: Don't use environment variables to store config info, use
>> static globals inside the modules instead]
> 
> I disagree: globals are almost as bad as environment variables. I think
> we should instead use context structures that we pass around, like geom
> does.

Not so.
I do not feel it makes much of a difference whether a variable resides
on the heap or somewhere in static data.

Context structures are essential when you need something more than once.
However, I don't see us configuring two wholly distinct partitionings
inside the same sysinstall session. It *IS* singleton state, and then
we might as well store it easily into static variables.

I do not like having an extra state parameter to worry about, and doing
state->root_partition is just that little bit more for my eyes to ignore
than reading just root_partition.

Of course, this is personal preference. But I do not believe (static)
globals should be unilaterally killed, they are quite useful. They have
these advantages over environment variables:
- arbitrarily typeable
- clear ownership/responsibility
- compiler enforces local-only access

Now, as long as the struct definition is local to the modules, and the
rest only passes around pointers, context structures provide the exact
same advantages. I fail to see any further advantages that would be
relevant for us: No relevant technological advantages, and nothing that
would force a better overall design/style, either.

Call me old-fashioned, but I don't like allocating memory dynamically
any more than I really have to. Having more large structures to
worry about makes me itchy. At the end of the day, it's all global
state - no matter if it is referenced as a static global field, or
as a field on a struct that was allocated exactly once and is
referenced a couple of times from the stack.

And don't tell me "but a struct can be free()d to reset to defaults" -
not so. The relevant "reset to defaults" happens inside your "initFoo",
and whether that allocates a new struct, and assigns defaults to its
field, or if it assigns the defaults to static fields, seems hardly
different at all.

So Far,
  Karsten


More information about the freebsd-sysinstall mailing list