On Sat, Sep 19, 2015 at 05:57:23PM -0400, Michael McConville wrote:
> If you're abstracting something into a function, it definitely shouldn't
> be creating more code.

Yet this shouldn't stop you from performing "divide and conquer". It's
not just about reducing lines of code when abstracting logical blocks,
i.e. functions.

If you want an example of what I mean, please look at the main function
of src/sbin/newfs_msdos/newfs_msdos.c.

> When you create functions for things like if (z) errx(...); conditions,
> it makes the code much harder to read.

Is that statement true for xmalloc(), too? ;)

On Sat, Sep 19, 2015 at 02:29:56PM -0400, Michael McConville wrote:
> -             sane_count(count);
> +             if (count < 0 || count >= PATH_MAX)
> +                     errx(1, "corrupted database");
>               for (p = path + count; (c = getc(fp)) > SWITCH; size++)

> -             sane_count(count);
> +             if (count < 0 || count >= PATH_MAX)
> +                     errx(1, "corrupted database");
>               /* overlay old path */
>               p = path + count;

I would recommend to replace PATH_MAX with sizeof (path) here, so it
looks less magical and adapts whenever someone changes path's size. I've
kept the very next line after your patches in my quote, which shows that
count is used as an index into path, therefore the sizeof (path) range
check.


Tobias

Reply via email to