Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NUL terminate "msg" in lessons 3 & 4 #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

joast
Copy link

@joast joast commented Jan 15, 2022

Terminate "msg" in lessons 3 and 4 with a NUL (0 byte) instead of relying on NUL padding inserted by the assembler to align things properly.

@agarmash
Copy link

I believe this is done on purpose. This issue is addressed later in lesson 6

@joast
Copy link
Author

joast commented Jan 16, 2022

I realize the issue is addressed in lesson 6 to fix the bug from lesson 5. However, I still believe it should be addressed in the string length lessons because the lessons are checking for a NUL byte, but one isn't explicitly included in the data area. This can be confusing to someone following the tutorial. i.e. "How can you look for something that isn't there?"

Also, if someone following the lessons decides to experiment and changes the message string to something that assembles without any padding, then they could have problems they aren't prepared to handle unless they read-ahead a few lessons.

@cassepipe
Copy link

Yay, as a beginner not having reaches lesson 6 yet, this was confusing. Hence my comment here and now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants