Patch refactor scm and textui #289

hanjiezhou · 2023-07-10T11:45:56Z

重构了屏幕管理器和textui框架

fslongjin · 2023-07-11T14:01:28Z

kernel/src/libs/libUI/screen_manager.rs

+    SCM_FRAMEWORK_LIST.lock().push_back(framework.clone());
+    // 调用ui框架的回调函数以安装ui框架，并将其激活
+    framework.install()?;
+    framework.enable()?;


为啥这里要调用enable，然后里面又调用1次

fslongjin · 2023-07-11T14:03:22Z

kernel/src/libs/libUI/screen_manager.rs

+pub extern "C" fn scm_enable_double_buffer() -> i32 {
+    let r = ture_scm_enable_double_buffer()
+        .map_err(|e| e.to_posix_errno())
+        .unwrap();


这里产生err的时候，直接unwrap会产生错误

fslongjin · 2023-07-11T14:03:22Z

kernel/src/libs/libUI/screen_manager.rs

+ */
+#[no_mangle]
+pub extern "C" fn scm_enable_double_buffer() -> i32 {
+    let r = ture_scm_enable_double_buffer()


true打错字啦，类似的地方还有好几处

fslongjin · 2023-07-11T14:06:38Z

kernel/src/libs/libUI/screen_manager.rs

+}
+/**
+ * @brief 销毁双缓冲区
+ *


类似这样的代码需要删掉

fslongjin · 2023-07-11T14:07:21Z

kernel/src/libs/libUI/screen_manager.rs

+        }
+    }
+
+    pub fn get_vaddr(&self) -> usize {


这种函数命名不符合规范，不需要get_前缀。

fslongjin · 2023-07-11T15:27:08Z

kernel/src/libs/libUI/textui.rs

+pub extern "C" fn scm_disable_put_to_window() {
+    // unsafe{kfree(*TEXTUI_BUF_VADDR.read() as *mut c_void)};
+
+    unsafe { ENABLE_PUT_TO_WINDOW = false };


原子变量

fslongjin · 2023-07-11T15:27:39Z

kernel/src/libs/libUI/textui.rs

+ * @return int
+ */
+#[no_mangle]
+pub extern "C" fn rs_textui_init() -> i32 {


导出到C的函数，都把他们丢到一个新的文件“c_adapter.rs"吧

fslongjin · 2023-07-11T15:28:00Z

kernel/src/libs/libUI/textui.rs

+
+    drop(private_info);
+
+    unsafe { TEST_IS_INIT = true };


这个干啥用的

fslongjin · 2023-07-11T15:31:05Z

kernel/src/libs/libUI/textui.rs

+pub static CHAR_PER_LINE: RwLock<i32> = RwLock::new(0);
+//textui 未初始化时直接向缓冲区写，不使用虚拟行
+pub static NO_INIT_OPERATIONS_LINE: RwLock<i32> = RwLock::new(0);
+pub static NO_INIT_OPERATIONS_INDEX: RwLock<i32> = RwLock::new(0);


这个变量我看的很迷惑，这个干啥用的？是未初始化内存管理的时候，用于记录下标的吗？

还有就是，我建议把内存管理未初始化的时候，相关的textui代码独立到一个新的文件"textui_mm_no_init.rs“，这样看的清楚一点。

还有就是，不要用rwlock去保护i32，直接原子变量就行了

fslongjin · 2023-07-11T15:31:31Z

kernel/src/driver/video/video.rs

@@ -0,0 +1,44 @@
+// use crate::include::bindings::bindings::PAGE_PCD;


删掉这个文件

kernel/src/libs/libUI/textui.rs

…nto zhouhanjie-patch_refactor_scm

fslongjin · 2023-08-02T13:38:20Z

docs/kernel/libs/libUI/scm.md

+&emsp;&emsp;用于储存scm中的ui框架中的帧缓冲区中的信息，定义如下：
+```rust
+#[derive(Debug, Clone)]
+pub struct ScmBufferInfo {


具体数据结构的内容我感觉可以不用放上文档，因为很难保持同步。只要说说是哪个结构就好，我觉得。然后文档主要讲实现思路（架构上面的），不用过于细节。因为细节的去看代码就能看的明白，因此我感觉把更高一个层次的视角的框架设计，写出来会更有用，下面那个文档也是一样的。

fslongjin · 2023-08-02T13:42:12Z

kernel/src/libs/libUI/screen_manager.rs

+}
+/// 允许往窗口打印信息
+#[no_mangle]
+pub extern "C" fn scm_enable_put_to_window() {


这个函数和下面的disable都不需要extern C

kernel/src/libs/spinlock.rs

fslongjin · 2023-08-02T13:50:36Z

kernel/src/libs/libUI/screen_manager.rs

+}
+
+#[derive(Debug, Clone)]
+pub struct ScmBufferInfo {


命名问题

这个结构体命名为ScmBuffer更合适

存在内存泄露问题

要为这个结构体实现Drop trait，在它被drop的时候释放缓冲区（如果是动态申请来的），否则将会造成内存泄露。

并且，由于在很多地方都直接clone这个结构体，然后把vaddr指向的内存当作缓冲区来使用，因此本身就存在大量的“越界访问”的隐患。上面derive的Clone需要删掉。

这个结构体命名为ScmBuffer更合适

fslongjin · 2023-08-02T13:51:58Z

kernel/src/libs/libUI/screen_manager.rs

+
+            frame_buffer_info.flags = buf_type;
+
+            frame_buffer_info.vaddr = VirtAddr::new(unsafe { kzalloc(video_frame_buffer_info.size as usize, 0) });


不要使用kzalloc申请内存，请使用allocate_frames，并且映射到指定的位置。

fslongjin · 2023-08-02T13:54:02Z

kernel/src/libs/libUI/screen_manager.rs

+    pub fn vaddr(&self) -> usize {
+        self.vaddr.data()
+    }
+    pub fn buf_size_about_u8(&self) -> u32 {


这个命名很有迷惑性，而且没有注释。需要更改。下面那个buf_size_about_u32同理

fslongjin · 2023-08-02T13:55:07Z

kernel/src/libs/libUI/screen_manager.rs

+}
+pub trait ScmUiFramework: Sync + Send + Debug {
+    // 安装ui框架的回调函数
+    fn install(&self) -> Result<i32, SystemError> {


这些函数全部都不需要默认实现吧，我感觉都是必须的？

fslongjin · 2023-08-02T13:55:38Z

kernel/src/libs/libUI/screen_manager.rs

+
+    let metadata = framework.metadata()?;
+
+    if metadata.buf_info.vaddr.data() == 0 {


调用vaddr的is_null方法，而不是这样写

fslongjin · 2023-08-02T13:56:51Z

kernel/src/libs/libUI/screen_manager.h

@@ -1,7 +1,14 @@
 #pragma once


这个文件我感觉可以删掉了

fslongjin · 2023-08-02T14:00:05Z

kernel/src/libs/libUI/textui.rs

+            Arc::new(SpinLock::new(TextuiWindow::new(
+                WindowFlag::TEXTUI_IS_CHROMATIC,
+                0,
+                0,
+            ))),
+            Arc::new(SpinLock::new(TextuiWindow::new(
+                WindowFlag::TEXTUI_IS_CHROMATIC,
+                0,
+                0,
+            ))),


current window为啥要和default的是不同的2个实例呀。。。
而且不应该先初始化line数量为0的window对象，然后再在外面的函数进行初始化赋值。这样不满足“资源获取即初始化”

fslongjin · 2023-08-02T14:02:42Z

kernel/src/libs/libUI/screen_manager.rs

+
+    // 遍历当前所有使用帧缓冲区的框架，更新地址
+    for framework in SCM_FRAMEWORK_LIST.lock().iter_mut() {
+        framework.change(unsafe { ScmBufferInfo::from(&video_frame_buffer_info) })?;


为什么要把所有的框架的缓冲区都设置成相同的？？？

fslongjin · 2023-08-02T14:04:37Z

kernel/src/libs/libUI/screen_manager.rs

+    // 遍历当前所有使用帧缓冲区的框架，更新地址
+    drop(scm_list);
+    for framework in SCM_FRAMEWORK_LIST.lock().iter_mut() {
+        (*framework).change(buf_info.clone())?;


这里也是，为什么要把所有框架的缓冲区设置成相同的？

fslongjin · 2023-08-02T14:07:12Z

kernel/src/libs/libUI/screen_manager.rs

+pub static SCM_DOUBLE_BUFFER_ENABLED: AtomicBool = AtomicBool::new(false);
+
+bitflags! {
+  pub struct ScmBfFlag:u8{


这个命名也需要改，很有迷惑性。一般不用bf来代指buffer的，这不是常用缩写

fslongjin · 2023-08-02T14:07:26Z

kernel/src/libs/libUI/screen_manager.rs

+}
+
+#[derive(Debug, Clone)]
+pub struct ScmBufferInfo {


这个结构体命名为ScmBuffer更合适

fslongjin · 2023-08-02T14:15:49Z

kernel/src/libs/libUI/screen_manager.rs

+    }
+}
+
+#[derive(Debug, Clone)]


由于buffer的内存安全问题，这个不应该继承clone

fslongjin · 2023-08-02T14:18:47Z

还有，textui.h里面有很多的无用的函数导出、无用的数据结构，这些都要删除

fslongjin · 2023-08-02T14:21:27Z

kernel/src/libs/libUI/textui.rs

+    /// 将窗口的帧缓冲区内容清零
+    pub fn _renew_buf(&self) {
+        // let mut addr: *mut u32 = fb as *mut u32;
+        let mut addr: *mut u32 = self.metadata.buf_info.vaddr() as *mut u32;
+
+        for _i in 0..self.metadata.buf_info.buf_size_about_u8() {
+            unsafe { *addr = 0 };
+            unsafe {
+                addr = (addr.offset(1)) as *mut u32;
+            }
+        }
+    }


感觉这个函数可以删掉

fslongjin · 2023-08-02T14:22:47Z

kernel/src/libs/libUI/textui.rs

+    fn change(&self, buf: ScmBufferInfo) -> Result<i32, SystemError> {
+        let src = self.metadata.buf_info.vaddr() as *const u8;
+        let dst = buf.vaddr() as *mut u8;
+        let count = self.metadata.buf_info.buf_size_about_u8() as usize;


这里有内存安全问题，没有检查src和dst是否同样大小。建议把拷贝缓冲区内容的工作，写成buffer的成员函数。

fslongjin · 2023-08-02T14:25:25Z

kernel/src/libs/libUI/textui.rs

+    ///  -失败：Err(错误码)
+    fn metadata(&self) -> Result<ScmUiFrameworkMetadata, SystemError> {
+        // let framework_guard = self.0.lock();
+        let metadata = self.metadata.clone();


这个函数会把buffer的vaddr给拷贝出去，这是有问题的，可能会导致难以定位的内存越界访问

fslongjin · 2023-08-02T14:26:33Z

kernel/src/libs/libUI/textui.rs

+    }
+}
+
+impl Deref for TextUiFramework {


这个deref和derefMut我建议删掉，因为他们允许外界对Metadata结构体的直接操作，破坏了封装性

hanjiezhou and others added 16 commits April 5, 2023 11:39

修改CFSqueue从Vec变成红黑树

4c9bc40

实现匿名管道

22a84b9

Merge branch 'master' into patch-refactor-pipe

b964e18

实现匿名管道

6d1e56f

实现匿名管道

d704f5b

实现匿名管道

e9774a8

增加IrqArch trait以及IrqFlags及其守卫

75aab4c

reformat code

3b891f1

实现匿名管道

0ad4957

未调试版

7f10e63

Merge branch 'master' into patch-refactor-pipe

9dfad06

未调试版

1722048

v1

d0fe573

重构scm与textui

bf2d7a2

Merge branch 'master' into patch_refactor_scm

d8ff201

重构scm与textui

6ecf0f7

fslongjin requested changes Jul 11, 2023

View reviewed changes

fslongjin reviewed Jul 12, 2023

View reviewed changes

kernel/src/libs/libUI/textui.rs Outdated Show resolved Hide resolved

fslongjin and others added 6 commits July 12, 2023 09:13

Merge branch 'master' into zhouhanjie-patch_refactor_scm

933622a

commit

480b647

Merge branch 'patch_refactor_scm' of github.com:hanjiezhou/DragonOS i…

fb28d9b

…nto zhouhanjie-patch_refactor_scm

稍微改了下，但是还有很多的要改的

c6a06e3

commit

2e41581

commit

a17c115

hanjiezhou closed this Jul 21, 2023

hanjiezhou reopened this Jul 21, 2023

hanjiezhou added 4 commits July 29, 2023 14:33

有bug

5beb195

ok

006dba9

Merge branch 'master' into patch_refactor_scm

3473038

ok

9fbafaf

ok

c72ea3f

fslongjin requested changes Aug 2, 2023

View reviewed changes

fslongjin added 2 commits August 2, 2023 14:02

reformat

4d5e25f

1

cfe57cd

fslongjin requested changes Aug 2, 2023

View reviewed changes

fslongjin reviewed Aug 2, 2023

View reviewed changes

hanjiezhou and others added 8 commits August 13, 2023 12:43

ok2

c3d47d1

ok

b404827

Merge branch 'master' into patch_refactor_scm

5375471

修改makeflie

f48e88c

解决初始化的时候由于页表已经有映射关系而报错的问题

b9adc40

切换字体为spleen，并增加对字体的抽象

4eac8f2

修正文档

9baea8c

设置printk writer，使用新的textui_putchar函数

f35fb8e

fslongjin approved these changes Aug 19, 2023

View reviewed changes

fslongjin merged commit abe3a6e into DragonOS-Community:master Aug 19, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Patch refactor scm and textui #289

Patch refactor scm and textui #289

hanjiezhou commented Jul 10, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Jul 11, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023 •

edited

Loading

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023 •

edited

Loading

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin commented Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

fslongjin Aug 2, 2023

		@@ -0,0 +1,44 @@
		// use crate::include::bindings::bindings::PAGE_PCD;


		frame_buffer_info.flags = buf_type;

		frame_buffer_info.vaddr = VirtAddr::new(unsafe { kzalloc(video_frame_buffer_info.size as usize, 0) });


		let metadata = framework.metadata()?;

		if metadata.buf_info.vaddr.data() == 0 {

Patch refactor scm and textui #289

Patch refactor scm and textui #289

Conversation

hanjiezhou commented Jul 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fslongjin Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

命名问题

存在内存泄露问题

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fslongjin Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fslongjin commented Aug 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fslongjin Aug 2, 2023 •

edited

Loading

fslongjin Aug 2, 2023 •

edited

Loading