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

[system] Major synchronization with ELKS updates in header file etc. #106

Merged
merged 2 commits into from
Nov 23, 2024

Conversation

Mellvik
Copy link
Owner

@Mellvik Mellvik commented Nov 22, 2024

Synchronize TLVC with a number of cleanups, adjustments, enhancements and more from ELKS header files and files that use them. Enables new functionality and some older updates to work with TLVC, and lays the groundwork for better portability between the two platforms.

@Mellvik
Copy link
Owner Author

Mellvik commented Nov 23, 2024

Also includes an important system enhancement in that bootopts may now be up to 1K bytes in size. With a seemingly continuous stream of important bootopts-options being added to the system, this enhancement allows keeping a minimum of comments in the file - preserving readability. Again, code from @ghaerr and ELKS.

@Mellvik Mellvik merged commit 279f3d6 into master Nov 23, 2024
o_seg = ebh->b_L2seg;
ebh->b_L2seg = current->t_regs.ds;
bh->b_data = (unsigned char *)buf;
o_seg = (seg_t)ebh->b_L2seg; /* may be long (xms active) or int */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken, this code stores 32-bits into 16-bits when XMS is active, since oseg is type seg_t (it should be ramdesc_t). In general, it is not recommended to use casts when copying variables unless absolutely necessary as this will hide any compilation warnings that otherwise might result.

ebh->b_L2seg = current->t_regs.ds;
bh->b_data = (unsigned char *)buf;
o_seg = (seg_t)ebh->b_L2seg; /* may be long (xms active) or int */
ebh->b_L2seg = (ramdesc_t)current->t_regs.ds;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same idea here. While not buggy, as 16 bits is copied to 32, the cast shouldn't be necessary. Did GCC output a warning here without the cast?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a great catch @ghaerri - a serious bug. When analysing it now, this will have to eventually crash the system when XMS is active. b_L2seg is being saved, then restored and the upper half will be gone when the buffer is returned to the free-list (XMS active).

I tested this just now, reducing the # of XMS buffers to 30 to force the issue and sure enough, the system goes dead when reusing that buffer. The reason it hasn't been caught yet is probably a combination of non-use (I haven't used XMS much for quite some time) and the huge # of XMS buffers we normally allocate. Plus, if there was a hang, it would not be easily repeatable and REALLY hard to catch.

Much appreciated.

@Mellvik
Copy link
Owner Author

Mellvik commented Nov 23, 2024

Thank you for taking the time - much appreciated!!

Apropos - when fine-reading some of the ELKS code, I found 2 completely unconsequential typos that you may be interested in:

  • signal.c line 149 (balance square bracket in printk)
  • kernel/sys.c line 306 (missing paren in commented out code)

@ghaerr
Copy link

ghaerr commented Nov 23, 2024

signal.c line 149 (balance square bracket in printk)

Thanks. Actually, that code was buggy and was fixed three days ago in ghaerr/elks#2113.

kernel/sys.c line 306 (missing paren in commented out code)

Wow, that must have been there for 20 years. Shows how often CONFIG_SUPPLEMENTARY_GROUPS has been used!

@Mellvik
Copy link
Owner Author

Mellvik commented Nov 23, 2024

signal.c line 149 (balance square bracket in printk)

Thanks. Actually, that code was buggy and was fixed three days ago in ghaerr/elks#2113.

interesting coincidence...

kernel/sys.c line 306 (missing paren in commented out code)

Wow, that must have been there for 20 years. Shows how often CONFIG_SUPPLEMENTARY_GROUPS has been used!

How about deleting?

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.

2 participants