I have no {} and I must scream...

Log in or register to post comments
Go To Last Post
17 posts / 0 new
Author
Message
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
#1
Posted by barnacle: Sat. Oct 5, 2019 - 08:32 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am currently trying to persuade a twenty-year-old assembler source to compile without warnings under GCC for 64-bit linux, and while I thank the original writer (it's available under GPL) for his work, I can't say the same for his style!

 

.h files? We've heard of them, but we don't use them, so it's very sensitive to the order in which the files are compiled. But there is no clue as to what the order should be... and it shouldn't matter.

 

How big is an int? I dunno... there's nothing in the code to indicate whether it's built on an 8, 16, 32 architecture. I can only vaguely guess that it probably wasn't 64 bits.

 

Magic numbers? They're really the only sort to use.

 

Externed variables? The only ones we've got.

 

Bizarre style? Obviously it's personal choice, but if only we all used the very sensible Allman style, and not one that looks like this:

			if (Find_Symbol(label) != UNKNOWN) {

				error_text = "Redefined label";
				error_flag = TRUE;
				}

			else

				Add_Symbol(label, Expression_Analyzer(operand));

Note the vertical spacing - there are loads of single-statement if and while statements, which usually (but not always!) have an empty line either side of the statement. He was also fond of single statements, without {} delimiters, that might have multiple nested switch, while or if statements within them. Trying to follow the flow is, um, challenging.

 

Can we guess what's wrong with this?

    *text = NULL;

We won't even discuss the oddball things like using non-standard library functions (strlwr() anyone?). As for returning from the middle of a case statement, not even bothering with a break, bah! And comments? Comments are for wimps! The code is self-documenting, obviously!

 

Sorry... I just needed a rant. But there is a serious point: if we all try and use standard C and not 'clever' coding; if we try and follow things like MISRA guides; if we think about future maintenance of the code... we might make life a ittle easier for all of us.

 

Neil

 

p.s. yes, I know - standards have changed over the years. But it's been obvious since the 80s that integer sizes would change with time; there's a good reason for stdint.h.

Neil Barnes
www.nailed-barnacle.co.uk

Tags:

Tools, Compilers and General Programming
Top
Bronze Markdavid.prentice
david.prentice's picture
Level: 10k+ Postman
Joined: Sat. Feb 12, 2005
Posts: 30995 View posts
Location: Wormshill, England
Posted by david.prentice: Sat. Oct 5, 2019 - 09:07 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Go on.   I can remember the 1980s.    All code for PCs was written as if big-endian did not exist.    All code assumed that pointers could be in a 16-bit int.

 

Ok,   the 1980s had the elegant M68000 CPU which was big-endian,  flat address space, ...

The PC world had the 8088 and 8086 with its attendant abortions.

 

Regarding blocks, indentation, style, ...

I suggest that you just run it through INDENT or similar formatter.    This will turn Double-Dutch into English.

 

Also from the 1980s.    There was an M68000 Assembler/disassembler/monitor/everything program that ran on an Atari ST.

The program was excellent.    I discovered at a later date that the Author had written the whole package in one ASM source file.

 

I never saw the source code.    But my admiration for the Author knew no bounds.     How could any single individual manage such a large file?

Of course the corollary was true.     No one could possibly succeed him or ever maintain the project.

 

Regarding 8-bit Cross-Assemblers.    There are hundreds of them.    Several were well written and maintainable.

 

David.

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sat. Oct 5, 2019 - 09:25 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0
 switch (thing)
 {
     case 1:
        ...
        return;
    case 2:
        ...
        return;
    case 3:
        ...
        return;
 }

No. Just no.

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top
N.Winterbottom
N.Winterbottom's picture
Level: Raving Lunatic
Joined: Tue. Oct 1, 2002
Posts: 2165 View posts
Location: Redbourn, Herts, UK
Posted by N.Winterbottom: Sat. Oct 5, 2019 - 10:03 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


barnacle in curmudgeon mode wrote:
Trying to follow the flow is, um, challenging.

When reading challenging block structure I would run the entire source (or a copy) through a source formatter which can remove blank lines and move braces. The Eclipse C/C++ formatter is good in that respect. I suspect INDENT or ASTYLE can also be coerced into doing that.

 

However no formatter will insert braces to make blocks more salient. In that case I recommend Netbeans which has a killer feature called "Breadcrumbs Navigator". The C/C++ plugin includes a provider for this feature.

 

Here it is in operation (in MPLAB X by-the-way, but that matters not)

 

 

 

As I said, it's also a navigator, so here's how after clicking the ELSE block, I find the matching THEN block.

 

 

Having used MPLABX for some years now I really miss this excellent feature in other IDEs.

 

Top
N.Winterbottom
N.Winterbottom's picture
Level: Raving Lunatic
Joined: Tue. Oct 1, 2002
Posts: 2165 View posts
Location: Redbourn, Herts, UK
Posted by N.Winterbottom: Sat. Oct 5, 2019 - 10:28 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Whist editing the mouse pointer into that last screenshot I see another whinge has appeared about early function returns.

 

Now I'm not going to start a formatting war (I've got better things to do this weekend) but I think this often contributes to shorter and easier to read code. Imagine there was a function tail you didn't want to run in those early return cases,  you'd have to write extra IF - THEN - ELSE on the switch variable or set a SKIP_FUNCTION_TAIL flag, neither of those is as helpful as an early return.

 

For some real life examples that have been peer reviewed, just pull up some random Linux code.

 

Oh! This was in my browsing history  https://github.com/torvalds/linux/blob/master/sound/hda/hdac_device.c

/* return CONNLIST_LEN parameter of the given widget */
static unsigned int get_num_conns(struct hdac_device *codec, hda_nid_t nid)
{
    unsigned int wcaps = get_wcaps(codec, nid);
    unsigned int parm;

    if (!(wcaps & AC_WCAP_CONN_LIST) &&
        get_wcaps_type(wcaps) != AC_WID_VOL_KNB)
        return 0;

    parm = snd_hdac_read_parm(codec, nid, AC_PAR_CONNLIST_LEN);
    if (parm == -1)
        parm = 0;
    return parm;
}

Oh Dear - Here we see use of unsigned int without explicit word length and early returns;

 

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sat. Oct 5, 2019 - 11:11 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hmm... it compiles clean, but doesn't work, so now the debugging starts.

 

#define TRUE -1
#define FALSE 0

N.Winterbottom, I hear your arguments about multiple exits, but I still prefer to follow the MISRA requirements of one entrance, one exit. You can set a return variable and break, to the same end (and I suspect the compiler will make the same code in either case). But using return to delimit a switch statement is beyond the pale.

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top
Bronze Markdavid.prentice
david.prentice's picture
Level: 10k+ Postman
Joined: Sat. Feb 12, 2005
Posts: 30995 View posts
Location: Wormshill, England
Posted by david.prentice: Sat. Oct 5, 2019 - 11:42 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It should be relatively easy to tidy up the code. It is only an Assembler after all.
.
Abandon the Assembler that you don't like. There should be several Cross Assemblers that you can choose from.
Which one upsets you?
.
David.

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sat. Oct 5, 2019 - 02:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This one is called 'fasm' - it's an assembler for 6502. There are surprisingly few assemblers around for forty year old processors that work on linux.

 

[That's not strictly true - there are quite a few around, but there don't appear to be many that just stick out a hex file at the end of the compilation, rather than some sort of .o file suitable for linking. Which is not my desire.)

 

But hey, I do this for fun :)

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top
Bronze Markdavid.prentice
david.prentice's picture
Level: 10k+ Postman
Joined: Sat. Feb 12, 2005
Posts: 30995 View posts
Location: Wormshill, England
Posted by david.prentice: Sat. Oct 5, 2019 - 02:31 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I have never heard of FASM

 

I would expect most Cross Assemblers to support 6502.   Most with an option to produce a .HEX file.

If not,  you only have to pass the .O file through a Linker.

 

Yes,  there might be minor syntax differences between Cross Assemblers e.g. the wedge syntax for HI8() or LO8() expressions.

 

Yes,  I would hope to translate the original 6502 SRC files.    If the Assembler needs some configuration to "suit" your 6502 Dialect it should be straightforward.

Yes,  I would prefer standard "MOS Technology" 6502 dialect.    Hey-ho,  we don't know what your SRC files look like.

 

David.

Top
N.Winterbottom
N.Winterbottom's picture
Level: Raving Lunatic
Joined: Tue. Oct 1, 2002
Posts: 2165 View posts
Location: Redbourn, Herts, UK
Posted by N.Winterbottom: Sat. Oct 5, 2019 - 04:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

barnacle wrote:
There are surprisingly few assemblers around for forty year old processors that work on linux.

You could have grabbed an old DOS 6502 assembler and run it under dosemu.

 

http://www.dosemu.org/

DOSEMU stands for DOS Emulation, and allows you to run DOS and many DOS programs, including many DPMI applications such as DOOM and Windows 3.1, under Linux.

In a previous life I ran BorlandC/C++ IDE for DOS using dosemu. It actually ran better there than I remember it did under real DOS 6.22.

 

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sat. Oct 5, 2019 - 05:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yeah, I've been down that route in the past: developing z80 code using Mame(?) to run cp/m tools natively...

 

I really ought to know better, but I would really expect C code to compile and run, even if designed for a different system/endianness etc. And I can fix this - at the moment, it's generating errors on every line, so now I'm learning my way around the eclipse/gdb operations to see what's going on.

 

My target code is actually a microsoft basic from back in the day - the 6502 source is available for a number of 70's dev systems, including to my surprise and delight, the Microtan 65 I learnt on all those years ago. I'd like to see the 8080 code too, but that looks a little harder to find (though I have found a few tiny basic sources).

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top
mike32217
mike32217's picture
Level: Hangaround
Joined: Sun. Feb 28, 2010
Posts: 472 View posts
Location: White Springs, FL
Posted by mike32217: Sat. Oct 5, 2019 - 05:53 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

barnacle wrote:

 switch (thing)
 {
     case 1:
        ...
        return;
    case 2:
        ...
        return;
    case 3:
        ...
        return;
 }

No. Just no.

 

Neil

Absolutely not...use goto instead of return!  :)

Happy Trails,

Mike

JaxCoder.com

Top
El Tangas
El Tangas's picture
Level: Raving Lunatic
Joined: Thu. Jul 14, 2016
Posts: 3496 View posts
Posted by El Tangas: Sat. Oct 5, 2019 - 06:11 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:

Whist editing the mouse pointer into that last screenshot I see another whinge has appeared about early function returns.

 

Now I'm not going to start a formatting war (I've got better things to do this weekend) but I think this often contributes to shorter and easier to read code. Imagine there was a function tail you didn't want to run in those early return cases,  you'd have to write extra IF - THEN - ELSE on the switch variable or set a SKIP_FUNCTION_TAIL flag, neither of those is as helpful as an early return.

 

 

I agree. I know functions with multiple return points are frowned upon, I really do, but in cases like the one you mention, it's the simplest and most readable solution IMO.

Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 5010 View posts
Location: SF Bay area
Posted by westfw: Sat. Oct 5, 2019 - 11:06 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

there might be minor syntax differences between Cross Assemblers

I had a horrible an entertaining time converting figforth 1.3 from whatever it had originally been written in to the TASM DOS cross assembler for Z80.

There were a bunch of obvious differences: "DW" vs ".DW" and similar, but the one that really had be scratching my head was that the original assembler supported

 DW     ZBRAN,LIST2-$

to define a couple of words in memory, but TASM would silently discard the second argument and only create a single word...

 

 

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sun. Oct 6, 2019 - 07:30 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Ah, the delights of

FILE * fopen (char * filename, char * mode)

where on MS systems, mode can include a 't' character to tell the file system that it's running on a text file, and causes line endings to get translated into \n whether they are \r\n, \r, or \n. That doesn't work on Linux systems, which only have the concept of  binary files.

 

So what is happening is that a line terminated \r\n (from the test file) is being seen as having an unhandled \r character on the end of it, confusing all sorts things but in particular the length of what should be empty lines... and as our friend is fond of constructs like

	if (label_length && valid_label)
	{

where the length is used directly as a logical value... yuck.

 

Easy quick fix is to change the line handler to look for \r as well as \n but on its own that leaves a lot of empty lines to confuse the listing, so still thinking about that.

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top
N.Winterbottom
N.Winterbottom's picture
Level: Raving Lunatic
Joined: Tue. Oct 1, 2002
Posts: 2165 View posts
Location: Redbourn, Herts, UK
Posted by N.Winterbottom: Sun. Oct 6, 2019 - 08:53 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You'll have found this StackOverflow question then:

https://stackoverflow.com/questions/5601581/how-to-detect-line-endings-across-text-files-from-different-os

Where the answer is to silently consume the '\r' by "peeking" ahead and finding a '\n'.

 

You can use fgetc/ungetc  to peek ahead.

 

Top
barnacle
barnacle's picture
Level: Raving Lunatic
Joined: Tue. Jan 3, 2006
Posts: 6803 View posts
Location: Berlin DE
Posted by barnacle: Sun. Oct 6, 2019 - 09:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I hadn't, but I've come up with a similar idea, modifying the existing get line routine. It already reads a character at a time to translate tabs '\t' into spaces, so I hooked in there...

 

int fgetln()
{
	char c = '\0';		//	NULL;
	char *string = current_line_buf;
	int length = 0;

	while ((c != '\n') && (c != EOF))
	{
		if (length < 256) 
		{
			c = fgetc(infile);
			switch (c)
			{
				case '\r':
					// silently eat it
					break;
				case '\t':
					// replace tab with space
					*string++ = ' ';
					length++;
					break;
				default:
					*string++ = c;
					length++;
					break;
			}
		}
	}
	// mark end of string
	*--string = '\0';		//	NULL;
	return(length);
}

Basically, turned an if/then into a switch and added some comments!

 

So now it's working, sorta. It generates correct code for the test file, produces a modified motorola s19 file, but for some reason prints out several thousand empty lines instead of a symbol table.

 

Neil

Neil Barnes
www.nailed-barnacle.co.uk

Top