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

begin kernel renaming. #99

Merged
merged 5 commits into from
Dec 17, 2016
Merged

begin kernel renaming. #99

merged 5 commits into from
Dec 17, 2016

Conversation

DaveeFTW
Copy link
Contributor

@DaveeFTW DaveeFTW commented Nov 20, 2016

remove all ForDriver and ForKernel suffix and prefix every kernel function with ksce instead of sce. Related issue #80

remove all ```ForDriver``` and ```ForKernel``` suffix and prefix every kernel
function with ```ksce``` instead of ```sce```. Related issue vitasdk#80
@xyzz
Copy link
Contributor

xyzz commented Nov 27, 2016

Is it possible that there are two functions named xxxForKernel and xxxForDriver with differing implementations?

@yifanlu
Copy link
Contributor

yifanlu commented Nov 29, 2016

@xyzz not that I've seen

@devnoname120
Copy link
Member

@DaveeFTW What's up about the renaming?

@devnoname120
Copy link
Member

db.json is left untouched because it will be removed anyway at some point.

Copy link
Contributor Author

@DaveeFTW DaveeFTW left a comment

Choose a reason for hiding this comment

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

Some corrections need to be taken in headers.

int sceBtStopAudioForDriver(int r0, int r1, int r2, int r3);
int sceBtStopInquiryForDriver(void);
int sceBtUnregisterCallbackForDriver(SceUID cb);
int ksceBtAvrcpReadVolumeForDriver(int r0, int r1, int r2, int r3);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove ForDriver suffix

@devnoname120 devnoname120 requested a review from yifanlu December 16, 2016 18:00
Copy link
Contributor

@yifanlu yifanlu left a comment

Choose a reason for hiding this comment

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

As discussed in IRC, I believe we should leave SceSysclibForDriver and SceDebugForDriver as is. At least for now, we can open up an issue and have farther debate about it.

@TheOfficialFloW
Copy link
Member

I am being conservative about this and have to say that I am totally against this prefix.
I don't see any reason to have suffix or prefix that indicate kernel functions. It has besn explained to me that the reason behind this is to be able to differentiate kernel functions from the user ones and thus developers won't get confused. But why would anybody pick the wrong functions when there's a separate folder called psp2kern for kernel headers? Also every kernel library has got the suffix ForKernel/ForDriver, that's enough of indication for the developer.
As a reverse engineer I'd like to have everything as original as possible. I don't think Sony named their functions with ForDriver/ForKernel.
In my opinion we should even omit it and let's have same names for kernel and user. One could also use two different db.yml for kernel and users if it's about being able to pick correct functions from the database.

Anyways, if you guys all really want to do it, I don't want to stop you, but I agree with @yifanlu that using the k prefix for syclib is a bad idea since one would need to rename everything when using open source libraries.

@DaveeFTW
Copy link
Contributor Author

@TheOfficialFloW Can you explain what you mean by "original as possible?" and provide an example where reverse engineering is hindered by not having an "original" name? My understanding is that we have almost zero "official" kernel names and unlikely to stumble across any more in the future. Striving for official names is unrealistic.

The SDK automatically links a significant portion of user libraries to assist developers. If we match names between user and kernel then kernel plugins will likely be compiled against user libraries or fail altogether without special flags. This can be frustrating to developer that are less experienced than yourself. If we have the correct flags, it is still possible for developers to link kernel libraries against user ones manually (or vice versa). Having a very simple namespace between the two APIs provides the needed abstraction to prevent all of this without significant re-engineering. This issue has already manifested several times throughout the beta period.

For future readers: the argument for dropping the prefix on sysclib for maintaining compatibility is a compromise. Using the sysclib functions is not recommended as we cannot (have not/will not) verify whether the implementations are actually conforming to the C standard or will/wont continue to in the future. memcpy in the past did not conform to the standard for example.
If you require access to libc-like functions in your plugin opt to use code from another source such as libk - which is a port of pdclib.

@TheOfficialFloW
Copy link
Member

@DaveeFTW I mean from the "reverse engineer's" point of view, not that it is hindering reverse engineers.

@devnoname120
Copy link
Member

devnoname120 commented Dec 16, 2016

@TheOfficialFloW
I was first also against this change, but finally decided that it was a good idea for multiple reasons. One reason is auto-completion. Some people use IDE's and rely on auto-completion to know what they can use, forcing them to look at the header files is annoying for them. Another reason is that a Kernel plugin should only contain k-prefixed functions, so this is way easier to spot incorrect uses, especially when copy-pasting code.

Also every kernel library has got the suffix ForKernel/ForDriver, that's enough of indication for the developer.

This is not true, there are many occurrences of Kernel-only functions that don't have the suffix: sceKernelLoadModuleForPid(), sceKernelRxMemcpyKernelToUserForPid(), and many more.

@TheOfficialFloW
Copy link
Member

@devnoname120 I mean the suffix of the stubs defined in Makefile.

@yifanlu
Copy link
Contributor

yifanlu commented Dec 16, 2016

@TheOfficialFloW so there's a couple of problems we are trying to solve with this change

  1. There is an inconsistency in how stuff is previously named. Kernel functions had either no suffix, or "ForDriver" or "ForKernel" at the end. The last two scheme was done arbitrarily by Proxima and I because we saw that the libraries had that suffix. However, in the case of SceKernelThreadmgr, notice that many of the NIDs for the ForDriver and ForKernel libraries are the same indicating that their original names were the same. Having both ForDriver and ForKernel is confusing in that respect.
  2. I don't like using the preposition "for" in these functions. Take the case of "sceKernelLoadModuleForPidForKernel" It indicates we are loading a module "for" another process but it is done "by" the kernel. Maybe we can do "sceKernelLoadModuleForPidByKernel" but then "sceIoOpenByKernel" sounds awkward. Having a prefix is easier to deal with in this case.

Your suggestion of just dropping prefix/suffix altogether doesn't work in all cases either. We have many user syscalls that can be called in kernel that might share the same name with a kernel function in kernel (SceCtrl is a good example, where the syscall version sees the flags differently). In these case, we might have both imports in the same application and therefore they require different names.

@yifanlu
Copy link
Contributor

yifanlu commented Dec 17, 2016

@TheOfficialFloW we will merge this unless you have any farther objections?

@TheOfficialFloW
Copy link
Member

@yifanlu
Well ok, you guys can do it.

@DaveeFTW DaveeFTW merged commit f47c196 into vitasdk:master Dec 17, 2016
@devnoname120 devnoname120 deleted the kernel_scheme branch April 5, 2017 15:43
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.

5 participants