usr/src/tools/btxld/btxld.c (Diff revision 1)

Show all issues

Do we need the explicit cleanup here? Isn't it going to be called via the atexit?

1. 

   Toomas Soome Nov. 28, 2018, 10 p.m.

   Well yes. I just figured that since the cleanup code was there, it might be good to keep it around - if for some reason we need to add some more activities, then we do not have to start to invent the cleanup...

2. 

   Robert Mustacchi Nov. 29, 2018, 3:07 p.m.

   OK, given that cleanup should be idempotent it should be OK I guess.

usr/src/tools/btxld/btxld.c (Diff revision 1)

Show all issues

It looks like this was always missing, right?

1. 

   Toomas Soome Nov. 28, 2018, 10:02 p.m.

   moved there from btxld(), as the original tempname cleanup was there.

Standard macro constant for maximum unsigned 32-bit quantity.

usr/src/tools/btxld/btxld.c (Diff revision 1)

Unnecessary, use UINT32_MAX from <stdint.h> instead.

usr/src/tools/btxld/btxld.c (Diff revision 1)

Should be:
if (sb.st_size > UINT32_MAX)

usr/src/tools/btxld/btxld.c (Diff revision 1)

Should be:
if (errno || !*arg || *s || x > UINT32_MAX)

Space-tab issues.

usr/src/tools/btxld/btxld.c (Diff revision 1)

Show all issues

Unnecessary space before tab.

usr/src/tools/btxld/btxld.c (Diff revision 1)

Show all issues

Same (+next line).

Ship It!

Unnecessary format string variables.

usr/src/tools/btxld/btxld.c (Diff revision 2)

I think these variables (binfo, cinfo, oinfo) unnecessary, because these format strings used only once (and far away).

usr/src/tools/btxld/btxld.c (Diff revision 2)

Could be:
    printf("kernel: ver=%u.%02u size=%x load=%x entry=%x map=%uM "
        "pgctl=%x:%x\n", btx.btx_majver, btx.btx_minver, btx.btx_textsz,
        BTX_ORIGIN(btx), BTX_ENTRY(btx),
        BTX_MAPPED(btx) * BTX_PGSIZE / 0x100000,
        !!(btx.btx_flags & BTX_MAPONE),
        BTX_MAPPED(btx) - btx.btx_pgctl -
        BTX_PGBASE / BTX_PGSIZE - BTX_MAPPED(btx) * 4 / BTX_PGSIZE);
    printf("client: fmt=%s size=%x text=%x data=%x bss=%x entry=%x\n",
        fmtlist[ihdr.fmt], ihdr.size, ihdr.text, ihdr.data, ihdr.bss,
        ihdr.entry);
    printf("output: fmt=%s size=%x text=%x data=%x org=%x entry=%x\n",
        fmtlist[ohdr.fmt], ohdr.size, ohdr.text, ohdr.data, ohdr.org,
        ohdr.entry);

This expression could be a good candidate for an another function-like macro:
BTX_MAPPED(btx) - btx.btx_pgctl - BTX_PGBASE / BTX_PGSIZE - BTX_MAPPED(btx) * 4 / BTX_PGSIZE

usr/src/tools/btxld/btxld.c (Diff revision 2)

Show all issues

Nested ternary conditional operator (c-style issue), could be:
    switch (i) {
        case I_LDR:
            fname = lname; 
            break;
        case I_BTX:
            fname = bname;
            break;
        default:
            fname = iname;
            break;
    }

Ship It!