Patches from the OpenBSD port of MultiMarkdown 4.7.1

[haqistan]

Hi! I've done a port of MultiMarkdown to OpenBSD so that it can be part of the standard set of 3rd party open-source packages available to all OpenBSD users. I love what you've done and use it on a daily basis - it has changed my feeling about LaTeX immeasurably.

Not only has it made me happier as a writer, it also introduced me to Parsing Expression Grammars, which impressed me so much I wrote a little blog post about it: http://trac.haqistan.net/blog/adventures-ports-parsing-expression-grammars

If for nothing else thanks for introducing me to this concept.

In doing the port I needed to make several patches, either for the sake of the OpenBSD ports build system or to quell warnings from the toolchain.

Makefile: The OpenBSD ports system doesn't really support git submodules; since I was interested in greg anyway I did an independent port of it (which has been accepted, my first!), so on OpenBSD I can make the MultiMarkdown port depend on the greg port and we can use the version of greg installed in /urs/local/bin. To make this work, I split GREG into OUR_GREG (greg/greg) and GREG (which defaults to ${OUR_GREG}) so that we can pass in GREG=/usr/local/bin/greg in the make invocation to pick up the installed version of greg without disrupting how the makefile works when you use the git submodule instead.

html.c: Conditionally switch from rand() et al to arc4random(3). For reference: http://marc.info/?l=openbsd-tech&m=141807224826859 is an email sent by Theo de Raadt to the tech@openbsd.org mailing list summarizing his research on rand(), srand(), et. al. OpenBSD has pioneered the use of arc4random() and it is a good idea to use it when available if you really want random numbers. I've added a HAVE_ARC4RANDOM #ifdef and turn it on from the OpenBSD port's makefile.

latex.c: Quell linker warnings on OpenBSD about strcat; it is a good idea to avoid it anyway but in this case it's actually shorter without strcat(), so double win.

parse_utilities.c: #ifdef out #pragma's that don't work under OpenBSD (#pragma mark is unsupported). While I was in there I also did some more arc4random() #ifdef's

parser.leg: Quell linker warning on OpenBSD about sprintf; snprintf() is safer.

writer.c: Similar changes as for latex.c: codeis shorter, no strcat warnings.

I hope you find at least some of these patches acceptable. In any event I'm glad to see you are actively developing MMD and hope you continue.

Cheers, -A

[fletcher]

Thanks for your interest in MMD and improving it, as well as migrating to the OpenBSD ports system!

I think you need to separate out your proposed changes:

Makefile: this change seems fine

html.c: Make sure to look at the actual code rather than dogmatically following someone else's recommendations. Think about it -- what does MMD actually need with high quality random numbers? It doesn't make use of cryptography and the like.... In fact, MMD by intent does not need random numbers, it needs pseudo-random numbers that can be reproduced again later in the same order. They don't need to pass any statistical tests of randomness, they just need to be scattered across a wide range. At best this change is unnecessary; at worst it breaks things. I would recommend undoing this change.

Similarly, are there problems with my use of strcat or sprintf? If there are bugs, please let me know. If it's simply a stylistic choice (or because other people have implemented these functions poorly), I don't see a reason to change.

If you're going to maintain this project moving forward, it would seem that the closer your codebase is to mine, the better. It will make merging future updates more reliable. That is, of course, up to you.

[haqistan]

Thanks for your response. I'll reply in-line:

html.c: Make sure to look at the actual code rather than dogmatically following someone else's recommendations. Think about it -- what does MMD actually need with high quality random numbers? It doesn't make use of cryptography and the like.... In fact, MMD by intent does not need random numbers, it needs pseudo-random numbers that can be reproduced again later in the same order. They don't need to pass any statistical tests of randomness, they just need to be scattered across a wide range. At best this change is unnecessary; at worst it breaks things. I would recommend undoing this change.

I came to the conclusion that you didn't want reproducible random numbers based on the arguments you gave to srand(3); looking at it closer I see you try to present the same argument to srand() for each level (and use the same random_seed_base), so this was just a misreading on my part - sorry. I was in fact a bit confused as to why you would want really random numbers, so I should've just looked closer to begin with.

I agree with what you say above, and in fact the email I pointed you at more or less says the same thing: most people don't realize what they are doing and crib from somewhere else. Moving forward I think the idiom that Theo and gang have proposed (srand_deterministic(3)) is clearer but it is not standard or available anywhere but OpenBSD yet.

In general I think it is always better to use idioms that declare to the reader what is intended; one of the points behind Theo's rant was that srand() does not communicate this because it is used both ways, e.g. srand(getpid()) vs. srand(known_value). In this case I made a mistake in diagnosing your intent.

Similarly, are there problems with my use of strcat or sprintf? If there are bugs, please let me know. If it's simply a stylistic choice (or because other people have implemented these functions poorly), I don't see a reason to change.

I would argue there is a problem with any use of strcat, strcpy or sprintf; that's why the toolchain under OpenBSD warns you of their presence. If only because it makes code harder to audit I and many others have come to the conclusion that these calls are best avoided. The strlcat, strlcpy and snprintf calls are always better alternatives because you must explicitly take the size of the buffer into account. It's not so much that your or my use of one of those functions is wrong as it is that someone else might cut and paste it into a context where it is wrong that bothers me about these calls. This is really where problems lie.

Nonetheless it is stylistic. I was being activist, but I'm not religious :-)

If you're going to maintain this project moving forward, it would seem that the closer your codebase is to mine, the better. It will make merging future updates more reliable. That is, of course, up to you.

Yes, of course. The Makefile patch really is the one that makes it easier for me to maintain the port. The other patches are not critical.

If you accept the Makefile patch I will ditch the rest of my patches in the port for the next update.

Thanks for your reply.