[Info-vax] openvms and xterm

Dave Froble davef at tsoft-inc.com
Fri May 3 22:58:17 EDT 2024


On 5/3/2024 9:04 PM, Dan Cross wrote:
> In article <v1366u$lf87$1 at dont-email.me>,
> Dave Froble  <davef at tsoft-inc.com> wrote:
>> :-)
>>
>> The following is some snippets from what was basically a research program, and I
>> consider commenting such should be rather terse.  For production programs I'd
>> want a lot more.

Do consider the above ...

> I hope you won't mind some feedback?

No problem.  But some background.  I came from an environment where strict 
programming practices were in effect.  Certain things, such as the header of a 
program file, had a strict format to follow, and much of the text came from an 
IDE code generator.  That allowed any support person to know where to look for 
information, and what to look for.

> Overall, there's a concept of, "too much of a good thing."
>
>> First, declare the purpose.  Shouldn't this always be done?
>
> Yes, but see below.
>
>>         !********************************************************************
>>         !
>
> What does the line of asterisks buy you?

Strict formatting in every program.  Important information was always highlighted.

>
>>         !       Program:        TCP_PEEK.BAS
>
> Why do you need this?  Is it not in a file that is already
> named, "TCP_PEEK.BAS"?  What do you get by repeating it in the
> file itself?

Doesn't hurt.

>>         !       Function:       Test Using TCP/IP Sockets as a Listener
>>         !       Version:        1.00
>>         !       Created:        01-Dec-2011
>>         !       Author(s):      DFE
>
> Why the elaborate formatting of this front matter?  What does
> some of it even mean?  How did you decide that this was version
> 1.00, for example?  (Something like semver is much more useful
> than an arbitrary major.minor here.)  Is the creation date
> useful versus, say, the last modification date?

For some, the first iteration of a program is version 1.  I guess other 
standards could be used.

Both dates can be useful.

> Moreover, where do you record the history of the module?
> Knowing how code has evolved over time can be very useful.

I didn't show that.  As I wrote, some snippets from a program, not the entire 
program.

> Frankly, all of this is metadata, which is better captured in a
> revision control system than in comments at the top of a file,
> which can easily get out of date over time.
>
>>         !
>>         !       Purpose/description:
>
> Well, which is it?

Now, that's a bit too "picky" ...

>>         !
>>         !               This program will set up TCP/IP sockets to allow
>>         !               itself to listen for connection requests.  When
>>         !               a connection request is received, this program
>>         !               will accept the connection, and then attempt to
>>         !               PEEK the message, ie; read it but leave it available
>>         !               to be re-read.
>
> This is good, but honestly, kind of all you need at the top of
> the file.
>
>> When using custom defined structures, it might be nice to know what they will be
>> used for.
>>
>>         !**************************************************
>>         !   Declare Variables of User Defined Structures
>>         !**************************************************
>>
>>         DECLARE IOSB_STRUCT     IOSB,                   !  I/O status blk &
>>                 ITEMLIST_2      SERVER.ITEMLST,         !  Server item list &
>>                 ITEMLIST_2      SOCKOPT.ITEMLST,        !  Socket options list &
>>                 ITEMLIST_2      REUSEADR.ITEMLST,       !  Reuse adr list &
>>                 ITEMLIST_3      CLIENT.ITEMLST,         !  Client item list &
>>                 SOCKET_OPTIONS  LISTEN.OPTN,            !  Socket options &
>>                 SOCK_ADDR       CLIENT.ADR,             !  Client IP adr/port &
>>                 SOCK_ADDR       SERVER.ADR,             !  Server IP adr/port &
>>                 BUFF            CLIENT.NAME,            !  Client name buffer &
>>                 BUFF            SERVER.NAME,            !  Server name buffer &
>>                 IP_ADR          IP,                     !  Ip address &
>>                 BUFF            MSG                     !  Message buffer
>
> I don't think that these comments are useful at all, with the
> possible exception of the one on IOSB.  These comments just
> parrot the code.  If I saw, "SOCKOPT.ITEMLIST" how is it not
> obvious that that is a, "Socket options list"?  Note also that
> the comment omits the fact that it's an item list, which is a
> specific thing, rather than just a generic list.  Half of the
> item lists are annotated as item lists in the comments, but
> the other half are not.
>
>> I consider the following rather terse.  Either the programmer knows how to use
>> system services, or perhaps remedial training is called for.
>>
>>         !**************************************************
>>         !        Assign channels to 'TCPIP$DEVICE:'
>>         !**************************************************
>>
>>         Dev$ = "TCPIP$DEVICE:"
>>
>>         Stat% = SYS$ASSIGN( Dev$ , ListenCh% , , )
>>         If      ( Stat% And SS$_NORMAL ) = 0%
>>         Then    E$ = FnVMSerr$( Stat% )
>>                 Print #KB%, "Unable to assign listener channel - "; E$
>>                 GoTo 4900
>>         End If
>>
>>         Print #KB%, "Internal VMS channel for listener socket:"; ListenCh%
>>
>>         Stat% = SYS$ASSIGN( Dev$ , ClientCh% , , )
>>         If      ( Stat% And SS$_NORMAL ) = 0%
>>         Then    E$ = FnVMSerr$( Stat% )
>>                 Print #KB%, "Unable to assign client channel - "; E$
>>                 GoTo 4900
>>         End If
>
> It's also rather repetitive.  It'd be better to wrap assignment
> in some kind of helper function, IMHO.  Without knowing more
> about VMS BASIC, however, it's difficult to tell whether one
> could do much better.
>
>> However, when details might be helpful, there is usually never too much.
>>
>>         !**************************************************
>>         ! Create Listener socket
>>         ! Bind server's IP address and port # to listener
>>         ! socket, set socket as a passive socket
>
> I'm not sure this comment is accurate: it appears that most of
> what this code is doing until the $QIOW is setting up data for
> the $QIOW.  Something like this may be more useful:
>
> ! Initialize IOSB data for listener socket creation,
> ! then create the socket and bind it to the server's
> ! IP address with the given port number.
>
>>         ! Note: we used to do this in 2 calls, but can be combined
>
> Grammar: "but they were combined."  Again, we see how missing
> the history can lead to questions: why were they combined, and
> when?  Is that better somehow, other than the general principle
> of doing less work?
>
>>         !**************************************************
>>
>>         LISTEN.OPTN::PROTOCOL% = TCPIP$C_TCP            !  Listener socket optn
>>         LISTEN.OPTN::TYP$ = Chr$(TCPIP$C_STREAM)
>>         LISTEN.OPTN::AF$ = Chr$(TCPIP$C_AF_INET)
>>
>>         SOCKOPT.ITEMLST::LEN% = 8%                      !  Socket options buffer
>>         SOCKOPT.ITEMLST::TYP% = TCPIP$C_SOCKOPT
>>         SOCKOPT.ITEMLST::ADR% = Loc(REUSEADR.ITEMLST::Len%)
>>
>>         REUSEADR.ITEMLST::LEN% = 4%                     !  Reuse adr (port #)
>>         REUSEADR.ITEMLST::TYP% = TCPIP$C_REUSEADDR
>>         REUSEADR.ITEMLST::ADR% = Loc(ReuseAdrVal%)
>>
>>         ReuseAdrVal% = 1%                               !  Set to 'True'
>>
>>         SERVER.ITEMLST::LEN% = 16%                      !  Server item list
>>         SERVER.ITEMLST::TYP% = TCPIP$C_SOCK_NAME
>>         SERVER.ITEMLST::ADR% = Loc(SERVER.ADR::Fam%)
>>
>>         SERVER.ADR::Fam% = TCPIP$C_AF_INET              !  Server Ip adr/port
>>         SERVER.ADR::PORT% = SWAP%(ServerPort%)
>>         SERVER.ADR::IP.ADR% = TCPIP$C_INADDR_ANY
>>         SERVER.ADR::ZERO1% = 0%
>>         SERVER.ADR::ZERO2% = 0%
>>
>>         BACKLOG% = 1%
>>
>>         Stat% = SYS$QIOW(       ,                       !  Event flag &
>>                                 ListenCh% By Value,     !  VMS channel &
>>                                 IO$_SETCHAR By Value,   !  Operation &
>>                                 IOSB::Stat%,            !  I/O status block &
>>                                 ,                       !  AST routine &
>>                                 ,                       !  AST parameter &
>>                                 LISTEN.OPTN::Protocol%, !  P1 &
>>                                 ,                       !  P2 &
>>                                 SERVER.ITEMLST::Len%,   !  P3 - local socket nam
>> e &
>>                                 BACKLOG% By Value,      !  P4 - connection backl
>> og &
>>                                 SOCKOPT.ITEMLST::Len%,  !  P5 - socket options &
>>                                 )                       !  P6
>>
>>         If      ( Stat% And SS$_NORMAL ) = 0%
>>         Then    E$ = FnVMSerr$( Stat% )
>>                 Print #KB%, "Unable to queue create and bind listener socket - "
>> ; E$
>>                 GoTo 4900
>>         End If
>>
>>         If      ( IOSB::Stat% And SS$_NORMAL ) = 0%
>>         Then    Stat% = IOSB::Stat%
>>                 E$ = FnVMSerr$( Stat% )
>>                 Print #KB%, "Unable to create and bind listener socket - "; E$
>>                 GoTo 4900
>>         End If
>>
>> My opinion is, the above is essential, without it, there would be much studying
>> of code, wondering what is being referenced, and such.  I always use one line
>> for each argument in a QIO and such, which makes it very clear what is
>> happening.  Without that, even  the best will still have some "fun" reading the
>> code to figure out what is happening.
>
> I agree that well-commented code is useful.
>
> 	- Dan C.
>


-- 
David Froble                       Tel: 724-529-0450
Dave Froble Enterprises, Inc.      E-Mail: davef at tsoft-inc.com
DFE Ultralights, Inc.
170 Grimplin Road
Vanderbilt, PA  15486



More information about the Info-vax mailing list