[Snark] C #define True "true"

Stephen Paul Weber singpolyma at singpolyma.net
Fri Nov 9 03:05:58 UTC 2018


>#define True "true"
>#define False "false"

I like this because it's many bad things at once.  It's a joke about 
beginner C89/90 users writing dumb "#define True 1", and an egregious case 
of using the wrong language feature for the job.  It's *also* a jab at how 
C99 has true/false keywords now.  I imagine it will also be used as the 
basis of a comment on code of the style `if(x == True)`.

>static char indexer_for_the_word_thingies = 1;

A lot of this one is based on variables having too-long names.  Also this 
one is a static mutable global, which just hurts me.

>static char ***A;

Keep typing stars until it compiles.

>struct __attribute__((packed)) thingimajigie {
>	char **start;
>	char *end;
>} word_thingies[256];

I heard packed is faster!  Make everything packed!

>int _strlen(char *s)
>{
>	return (s ? strlen(s) : 0);
>}

This is not an obviously good idea, but it's not the worst idea in this 
code.

>void load_me_the_thingie_withie(int charie, char *thisie) {

Might be pushing it with the "ie"s here.

>	unsigned char i = ((2)<<6);

All the best C code has a bit shift.

>	do {
>		void *S = &(word_thingies[i]);

&(word_thingies[i]) === (word_thingies + i)

>		if (i < charie) {
>			((struct thingimajigie *)S)->start = &(*A)[i];

I'm like, why are we casting?  Oh... we made S void for no reason.
Also, we haven't set A in this code at all, so we're definitely relying on 
someone having set up this global for us, which is not at all obvious.

>			((struct thingimajigie *)S)->end = malloc(_strlen((*A)[i])+1);

Ok, so, start is a pointer to something that was in A... end is newly 
allocated space?

Also, we use the result of malloc without checking here...

>			memset((((struct thingimajigie *)S)->end), 'A', _strlen((*A)[i]));

Make the new buffer... all As?

>			*((char*)((struct thingimajigie *)S)->end+strlen((*A)[i])) = '\0';

Oh, so *now* it's ok to use default strlen?  ...one the same value we've 
been using our custom one on...

Also, *(x+y) === x[y]

>			S -= (sizeof(char**)+sizeof(char*));

Right. Why use the size of our elements when we can assume how the size of 
those elements is constructed?

>/* KITTEN <-> CAT <-> CONCATENATE... it's a bad joke don't @ me */

When your code is so bad you document it.

>#define KITTEN(a, b) a # b
>#define BALL_OF_STRING(a,b) a ## b
>#define DEFISCHARACTER(string) size_t BALL_OF_STRING(is, string) (int c) { return (c == KITTEN(, string)[0] || tolower(c) == KITTEN(, string)[0]) ? 1 : 0; }

There are lots of great little bad things here, but the theme is "abuse 
macros for no reason".  Also, the code the macros expand to isn't great, 
comparing an int that I assume represents a character with the first char in 
a string literal, doing the comparison again with tolower on the int and the 
same string literal again.. and the classically bad code `cond ? 1 : 0`

>char *isvowelay(char S, char S1) /* REMOVE: USE LIKE: S = isvowelay(string[0], string[1]); strcmp(S, "true");  */

Comment that tells you to remove it.  Function whose use is so non-obvious 
that you have to document it.

>	#define ISVOWEL(_) (isa(_) || ise(_) || isi(_) || iso(_) || isu(_))

Naming the macro argument `_` is a nice touch.  Though, why doesn't this 
macro return True or False? ;)

>struct capture isconsonant(char *string)

Besides formatting issues, this function is actually kind of fine.

Oh, it re-uses a macro that was defined "inside" a different function above, 
which is definitely confusing but more a problem with the definition above.

>	struct capture c = {.i = 0, .j = 0};

I guess the capture indices could have better names.  While many names in 
this program are uselessly long, these ones are not descriptive enough.

>void write_string(char *dest, char *src, struct capture c)
>{
>	strncpy(dest, src+c.i, c.j);
>}

Friends don't let friends use strncpy.  Especially not without a 
null-termination addition or length check.

>void dologic(char *S, char *dest)

Nice.

>	if (strcmp(True, isvowelay(S[0], S[1])) == 0) {

Here it is.  The glorious `strcmp(True, _) == 0`.  It's even better because 
you need *two* comparison checks (strcmp and == 0).  At least it's not 
`strcmp("true", ...`

>		write_string(dest+a.j, "ay\0", b);

Nice gratuitous NUL you got there.

>	/* printf("c.i: %d; c.j: %d\n", c.i, c.j); */

Commented out debug code, just in case you've never heard of version 
control.

>		a.i = c.j;
>		a.j = strlen(S)-c.j;
>
>		b.i = 0;
>		b.j = c.j;
>
>		d.i = 0;
>		d.j = 3;

So, while some of the less-descriptive names aren't too bad, this works out 
quite well.  Reading this code by itself, you have no idea whatsoever what 
it does.

>int people_gave_one_argument_with_the_words_rather_than_giving_them_as_separate_arguments()

Back to giant identifiers.  A bit surprised it's not `_argumentsie` ;)

>	for (i = 0; i < strlen(*((*A)+1)); i++)

This strlen call is atrocious, though mostly as a result of bad design 
choices elsewhere.

>		if (((*A)[1])[i] == ' ' && isalpha(((*A)[1])[i+1]))
>			return 1;
>	}
>	return -1;

Why use our defined True and False when 1 and ... -1? Will do.  That's 
right, this is a *second* redefinition of how bools should work.

>int main(int _, char **__)

More underscore names.

>	A = &__;

Type & until it compiles!

>		/* I never designed for this shit, only noticed when it came to the tests, so instead let's design around it */
>		char *command = calloc(strlen(__[0]) + strlen(__[1]) + 2, sizeof(char));
>		strcpy(command, __[0]);
>		command[strlen(__[0])] = ' ';
>		strcpy(command+1+strlen(__[0]), __[1]);
>		// printf("command: %s\n", command);
>		FILE *f = popen(command, "r");
>		int i;
>		do {
>			fputc((i = fgetc(f)), stdout);
>		} while (!feof(f) && i != '\n');
>		fclose(f);
>		free(command);
>		return 0;

This is actually great.  Instead of parsing the phrase argument, use
terrible hacks to call ourselves with each word as a seperate argument.
Don't pass the subprocess STDOUT through directly, though, copy it 
byte-by-byte manually instead.

Also calloc.  And without checking the result.  And more commented out debug 
code.

>		printf("%s", word_thingies[i].end);

Never heard of puts.

>	fputc('\n', stdout);

Never heard of putchar.

Also a contender, depending on the kind of badness we want to highlight this 
month.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://snark.badcode.rocks/pipermail/snark/attachments/20181108/91fd2a40/attachment-0001.sig>


More information about the Snark mailing list