Usage of sizeof('\0') for computing the size of string terminators

[fstromback]

Hi!

I am maintaining the Debian package storm-lang (for the language Storm) that uses MPS. I just received a bug report related to code in the MPS. In particular, the bug report points out that the files code/event.h and code/eventcom.h both use sizeof('\0') to compute the size of the null-terminator in a null-terminated string. When I test this on my system, sizeof('\0') evaluates to 4, which does not look like the intended behavior. This is, however, not critical as the only effect is that "too much" memory is reserved for some buffers (if even that, due to alignment).

The bug report (linked below) proposes a fix for this issue. I am happy to submit it as a pull-request if it helps you. However, since the MPS is well-engineered I figured there might be a reason for this seemingly odd code that I have missed (either a technical one, or related to style).

The bug report is here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1058778. I have not yet tested the proposed fix, but I am happy to do so if it helps.

[NickBarnes]

Curious. IIRC character constants like '\0' have type int, so will often have size 4, say, and (I think) can't have size 1 on "hosted" implementations.

[fstromback]

You are right. And that is the "bug" that has been reported.

In the code, the expression sizeof('\0') is used to compute the maximum length of a string (EventStringLengthMAX + sizeof('\0')). As such, it looks like the intent of the code is to add 1 to EventStringLengthMAX to account for the 0-terminator in the string.

As you point out, however, this is not what happens since sizeof('\0') is sizeof(int) which is larger than 1. This is why the bug-report was filed in Debian - the code as it is written looks like it wishes to add 1, while that is not what is happening.

I am not familiar enough with the event part of the MPS code, so I don't know if it is intended to allocate "too much" for the event strings (e.g. to keep alignment), or if it is indeed a case where 3 (or 7) bytes more than intended are reserved by using sizeof. If the intent was to add 1, then maybe it is beneficial to use some other way of indicating "one byte for a 0-terminator". On the other hand, if it is important that more than one byte is reserved, then maybe that should be clarified somehow?

[rptb1]

I'm 99% certain this is a mistake on my part -- trying to express the intention of adding one to a length (to hold a NUL).

In any case the fact that @fstromback isnt' sure of the interpretation that this is adding space for a NUL means the code violates rule.generic.clear.

Incidentally, the Debian bug report says:

The website is an information black hole, didn't find a mail address there, so submitting here.

@fstromback do you know which website?

[rptb1]

The patch suggested in the Debian bug report looks fine to me. You're welcome to send us a pull request, @fstromback . It would help to exercise our code-review-with-external-people procedure! (Or we'll get to it anyway.)

[rptb1]

I'm not serious, but we could say sizeof("") 😄

[fstromback]

Thank you for looking at it! I will prepare the patch as a pull-request shortly.

Regarding the website: No, I don't know if the submitter means the webpage for Storm (which has an e-mail on the bottom of the first page, but no obvious "send patches here"), or if they mean the MPS page. I can try asking them.

[fstromback]

Also: after thinking about it, I like the proposal of sizeof("") 😁 I did, however, have to think about it before realizing that it would not evaluate to sizeof(void *).. Nice one!

[rptb1]

It's fun but it violates rule.code.tricky exactly because you had to think about it :)

[fstromback]

I received a reply regarding which website was referred in the original bug report. Apparently it is the one for Storm.