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

add feature :dump dtb files #16

Closed
wants to merge 1 commit into from
Closed

Conversation

toneywoo
Copy link

dump dtb files from image

Copy link
Owner

@7Ji 7Ji left a comment

Choose a reason for hiding this comment

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

Sorry but I would not pull the current commit:

  1. This builds with warning, please fix them:
    cc -c -o obj/dtb.o src/dtb.c -Iinclude -Wall -Wextra
    src/dtb.c: In function 'dtb_read_into_buffer_helper':
    src/dtb.c:547:22: warning: comparison of integer expressions of different signedness: 'int' and 'uint32_t' {aka 'unsigned int'} [-Wsign-compare]
    547 |         for(int i=0;i<mhelper.entry_count;i++){
        |                      ^
    src/dtb.c:563:21: warning: unused variable 're' [-Wunused-variable]
    563 |             ssize_t re = write(fdtb,mhelper.entries[i].dtb,mhelper.entries[i].size);
        |                     ^~
    
  2. Why do you initialize a string or char array like this?
    char sname[256] = {""};
    
    If you consider it a string, initialize it with ""; if you consider it a char array, initialize it with {0}
  3. The stdout is dedicated for snapshot output in dsnapshot, esnapshot and webreport mode, please don't use it for log
  4. Why did you open file as fdtb then check the fd?
  5. Why do you set the return value as re and does not do any check on it? If you're not gonna perform any checks then don't even set it.
  6. Why are there commentted codes?
  7. Please leave a space between variable, operator and operands. Keeping them as compact as this makes them very hard to read and follow.
  8. You shouldn't introduce the logic to this part unconditionally, either. At least add a new flag and let the user define the output folder.

@7Ji
Copy link
Owner

7Ji commented Jan 11, 2024

Close as no new commit requested after 2 weeks

@7Ji 7Ji closed this Jan 11, 2024
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