On 09/14/17 03:17, Wang, Jian J wrote: > Thanks for the comments and good advices. Sorry the format issues. > This is my first patch for this project. Too many details for me to get > familiar with. > > (1) Sure. > (2) I'll change that. > (3) I'll use the tool to ensure the patch format. > (4) I'll remove the ',' in name > (5) I'll add more description about it. > (6) You're right. I should use SetMemorySpaceAttributes() of DXE service > instead. The only reason I didn't do it is that I found > GetMemorySpaceDescriptor() doesn't return the same information > which SetMemorySpaceAttributes() just changed. So I feel using CPU > arch protocol is a bit safer. Anyway, I'll change it. > (7) I did put those macros in the install function before. To reduce the > number of changed files, I made current changes. You're right it's > not worthy. > (8) Using macro can help the readability, which is more important to me. > I know function can do the same. But it looks a bit heavy in this > situation. > I have to admit replacing the macros with a library is a very good idea, > > which brings the same readability. I didn't think of that before. > Although > Library is still a little bit heavy to me but it's in a different way, I > think it > worth a trying. > (9) Putting a space before open parenthesis is forced style? If so, I'll add > it.
Yes, it is in the CCS: https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/52_spacing.html#52-spacing > 5.2.2.6 Always put space before an open parenthesis > > The only exception is macro definitions. > > if (... > while (... > EfiLibAllocateCopyPool (... Thanks, Laszlo > (10) You're right. Using library can reduce the disturbs to affected drivers > by this feature to the minimum. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
