-
Notifications
You must be signed in to change notification settings - Fork 0
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
Conversation
Also includes an important system enhancement in that |
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 */ |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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:
|
Thanks. Actually, that code was buggy and was fixed three days ago in ghaerr/elks#2113.
Wow, that must have been there for 20 years. Shows how often CONFIG_SUPPLEMENTARY_GROUPS has been used! |
interesting coincidence...
How about deleting? |
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.