On Wed, Jan 11, 2023 at 2:32 AM 'enh' via Android Building < [email protected]> wrote:
> On Tue, Jan 10, 2023 at 5:17 PM Michael Goffioul < > [email protected]> wrote: > >> Well, it turns out I believe there's invalid code in the external camera >> provider HAL, which happens to work only on 32 bits. More specifically, >> this snippet: >> https://cs.android.com/android/platform/superproject/+/master:hardware/interfaces/camera/provider/2.4/default/ExternalCameraProviderImpl_2_4.cpp;l=44-47 >> >> const char* kDevicePath = "/dev/"; >> constexpr char kPrefix[] = "video"; >> constexpr int kPrefixLen = sizeof(kPrefix) - 1; >> constexpr int kDevicePrefixLen = sizeof(kDevicePath) + kPrefixLen + 1; >> >> The above constants are used in the code to locate the numerical value in >> the video device name, e.g. "2" in "/dev/video2". The constant kPrefixLen >> is computed from constant char array kPrefix, minus one, as the array will >> have the terminating null byte (which doesn't count when computing the >> digit offset), that is 5. But then, kDevicePrefixLen does not make sense: >> - first it uses "sizeof(kDevicePath)", which is the size of a pointer; >> that is 4 on 32 bits, 8 on 64 bits >> > > no, that's weird -- and i tell people not to do it in code review because > it's confusing -- but it's correct because it's an _array_ rather than a > char*. > With all due respect, I think you may be mistaken here. The constant kDevicePath is not declared as an array, but as a const char*, so sizeof(kDevicePath) is the size of a pointer. E.g. try to compile the following in 32 or 64 bit: #include <iostream> const char* kDevicePath_1 = "/dev/"; constexpr char kDevicePath_2[] = "/dev/"; int main(int argc, char** argv) { std::cout << "sizeof(kDevicePath_1) = " << sizeof(kDevicePath_1) << std::endl; std::cout << "sizeof(kDevicePath_2) = " << sizeof(kDevicePath_2) << std::endl; return 0; } I get the following results (note: I've tried with both gcc - from Fedora 37 - and clang - from AOSP prebuilts @ android-13.0.0_r24): $ g++ -m32 -o testsize testsize.cpp $ ./testsize sizeof(kDevicePath_1) = 4 sizeof(kDevicePath_2) = 6 $ g++ -o testsize testsize.cpp $ ./testsize sizeof(kDevicePath_1) = 8 sizeof(kDevicePath_2) = 6 > (it's actually 6 in both cases, because it also includes the '\0', which > is one reason why people use it rather than the more obvious `strlen() + > 1`.) > But let's still assume I'm wrong, and that `sizeof(kDevicePath)` does indeed yield 6, like you said. Then kDevicePrefixLen would be 12. However the wanted value is the length of the string "/dev/video", that is 10. That is, kDevicePrefixLen would have an incorrect value. > what's the actual failure you're seeing? > First let me stress the fact that I'm compiling the external camera provider HAL in 64 bits mode (I'm targeting a 64-bits only build). I have 2 USB cameras attached to the device, using /dev/video0 and /dev/video2. Both devices are reported to cameraserver with the same identifier [email protected]/external/0, so basically cameraserver only sees 1 camera device. I believe that's due this snippet in ExternalCameraProviderImpl_2_4::addExternalCamera: std::string cameraId = std::to_string(mCfg.cameraIdOffset + std::atoi(devName + kDevicePrefixLen)); and the fact that std::atoi returns 0 if no conversion can be made. As a side note, the above snippet also performs an out-of-bound array access on 64 bits platform. Granted, the external camera service is currently forced into 32 bits compilation in AOSP, so this out-of-bound access is only latent. Michael. -- -- You received this message because you are subscribed to the "Android Building" mailing list. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/android-building?hl=en --- You received this message because you are subscribed to the Google Groups "Android Building" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/android-building/CAB-99LvLjaNO3yFP%3Dt7MuP6BAo71UqyEictSpCdVWWOXNZvhyA%40mail.gmail.com.
