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, 2 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, 7:07 a.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, 2: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!