On Wed, Jan 11, 2023 at 2:32 AM 'enh' via Android Building < android-building@googlegroups.com> wrote:
> On Tue, Jan 10, 2023 at 5:17 PM Michael Goffioul < > michael.goffi...@gmail.com> 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 device@3.4/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 android-building@googlegroups.com To unsubscribe from this group, send email to android-building+unsubscr...@googlegroups.com 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 android-building+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/android-building/CAB-99LvLjaNO3yFP%3Dt7MuP6BAo71UqyEictSpCdVWWOXNZvhyA%40mail.gmail.com.
