Some cosmetic suggestions for kx10_imp.c

[afberendsen]

Hi.

I have some suggestions for your code:

I. Consistent IMP commands help page

From

#if KS
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "addr", "addr",  &uba_set_addr, uba_show_addr,
            NULL, "Sets address of IMP11" },
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "vect", "vect",  &uba_set_vect, uba_show_vect,
            NULL, "Sets vect of IMP11" },
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "br", "br",  &uba_set_br, uba_show_br,
            NULL, "Sets br of IMP11" },
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "ctl", "ctl",  &uba_set_ctl, uba_show_ctl,
            NULL, "Sets uba of IMP11" },
#endif

To

#if KS
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "ADDR", "ADDR",  &uba_set_addr, uba_show_addr,
            NULL, "Sets IMP address." },
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "VECT", "VECT",  &uba_set_vect, uba_show_vect,
            NULL, "Sets IMP vector." },
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "BR", "BR",  &uba_set_br, uba_show_br,
            NULL, "Sets IMP BR." }, 
    {MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "CTL", "CTL",  &uba_set_ctl, uba_show_ctl,
            NULL, "Sets IMP UBA controller (UBA#1 or UBA#3)." },
#endif

This will help to make the help message to be consistent:

set IMP MAC=xx:xx:xx:xx:xx:xx   MAC address
set IMP IP=ddd.ddd.ddd.ddd/dd   IP address
set IMP GW=ddd.ddd.ddd.ddd      Gateway address
set IMP HOST=ddd.ddd.ddd.ddd    HOST IP address
set IMP NODHCP                  Don't aquire address from DHCP
set IMP DHCP                    Use DHCP to set IP address
set IMP UNI                     Standard Unibus transfers
set IMP SIMP                    PDP10 byte order transfers
set IMP ARP=ddd.ddd.ddd.ddd=XX:XX:XX:XX:XX:XX
                                Create a static ARP Entry
set IMP addr=val                Sets address of IMP11
set IMP vect=val                Sets vect of IMP11
set IMP br=val                  Sets br of IMP11
set IMP ctl=val                 Sets uba of IMP11
set IMP ENABLE                  Enables device IMP
set IMP DISABLE                 Disables device IMP
set IMP DEBUG                   Enables debugging for device IMP
set IMP NODEBUG                 Disables debugging for device IMP

II. Better erro formatting

The error messages below are missing a new line (\n), at the end. Currently, after the error message is displayed, the SCP prompt is dispaplyed on the same line as the error line.

t_stat imp_set_arp (UNIT* uptr, int32 val, CONST char* cptr, void* desc)
{
    char abuf[CBUFSIZE];
    in_addr_T ip;
    ETH_MAC mac_addr;

    if (!cptr) return SCPE_IERR;

    cptr = get_glyph (cptr, abuf, '=');
    if (cptr && *cptr) {
        if (SCPE_OK != eth_mac_scan (&mac_addr, cptr)) /* scan string for mac, put in mac */
            return sim_messagef(SCPE_ARG, "Invalid MAC address: %s", cptr);
    } else
        return sim_messagef(SCPE_ARG, "Invalid MAC address: %s", cptr);
    if (ipv4_inet_aton (abuf, (struct in_addr *)&ip)) {
        imp_arp_update(&imp_data, ip, &mac_addr, ARP_DONT_AGE);
        return SCPE_OK;
    }
    return sim_messagef(SCPE_ARG, "Invalid IP Address: %s", abuf);
}

III. Improved error messages

Because of function processing, the error messages are receiving a null string at cptr. So, when an error is detected, the %s gets a null string, instead of the invalid string of characters.

sim> set imp arp=1.2.3.4
%SIM-ERROR: Invalid MAC address: sim> exit
Goodbye

t_stat imp_set_arp (UNIT* uptr, int32 val, CONST char* cptr, void* desc)
{
    char abuf[CBUFSIZE];
    in_addr_T ip;
    ETH_MAC mac_addr;

    if (!cptr) return SCPE_IERR;

    cptr = get_glyph (cptr, abuf, '=');
    if (cptr && *cptr) {
        if (SCPE_OK != eth_mac_scan (&mac_addr, cptr)) /* scan string for mac, put in mac */
            return sim_messagef(SCPE_ARG, "Invalid MAC address: %s", cptr);
    } else
        return sim_messagef(SCPE_ARG, "Invalid MAC address: %s", cptr);
    if (ipv4_inet_aton (abuf, (struct in_addr *)&ip)) {
        imp_arp_update(&imp_data, ip, &mac_addr, ARP_DONT_AGE);
        return SCPE_OK;
    }
    return sim_messagef(SCPE_ARG, "Invalid IP Address: %s", abuf);
}

[rcornwell]

Thank you, I applied changes. I will push them to main simH tree in a couple of days.

[afberendsen]

I just remembered of one more...

From:

MTAB imp_mod[] = {
    { MTAB_XTD|MTAB_VDV|MTAB_VALR|MTAB_NC, 0, "MAC", "MAC=xx:xx:xx:xx:xx:xx", &imp_set_mac, &imp_show_mac, NULL, "MAC address" },
#if MPX_DEV

To:

MTAB imp_mod[] = {
    { MTAB_XTD|MTAB_VDV|MTAB_VALR|MTAB_NC, 0, "MAC", "MAC=xx:xx:xx:xx:xx:xx[/bits][>file]", &imp_set_mac, &imp_show_mac, NULL, "MAC address" },
#if MPX_DEV

[afberendsen]

...and another one...

From

    { MTAB_XTD|MTAB_VDV|MTAB_VALR, 0, "IP", "IP=ddd.ddd.ddd.ddd/dd", &imp_set_ip, &imp_show_ip, NULL, "IP address" },

To:

    { MTAB_XTD|MTAB_VDV|MTAB_VALR, 0,  "IP", "IP=ddd.ddd.ddd.ddd[/dd]", &imp_set_ip, &imp_show_ip, NULL, "IP address. Default mask is /32." },

[rcornwell]

can we close this issue?