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*. (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`.) > - second it adds one to the invalidly computed length of "/dev/video" (I'm > assuming that was the intent of the author) > Assuming the author thought "sizeof(kDevicePath)" would yield the length > of the constant string "/dev/" (I think that a reasonable assumption), then > incrementing by one "sizeof(kDevicePath) + kPrefixLen" does not make sense, > as the non-incremented value is what you're after. But on 32 bits, > something funny happens: the length of "/dev/" is 5, and > sizeof(kDevicePath) is 4. Maybe the author noticed that kDevicePrefixLen > didn't have the expected value, then noticed the previous line where > kPrefixLen is decremented by one, thought kPrefixLen should then be > re-incremented, and Bob's your uncle. Works fine on 32 bits, falls flat on > 64 bits. > what's the actual failure you're seeing? > Not sure if anybody at Google is reading this, but you might wanna have a > look at that more closely. > > Michael. > > > > > On Thu, Sep 29, 2022 at 3:09 PM Michael Goffioul < > [email protected]> wrote: > >> Is there a particular reason to keep the external camera service a 32-bit >> only component [1]? From what I can see, the cameraserver process has been >> made available for 64-bit build [2]. >> >> Thanks, >> Michael. >> >> [1] >> https://cs.android.com/android/platform/superproject/+/master:hardware/interfaces/camera/provider/2.7/default/Android.bp;l=62 >> [2] >> https://cs.android.com/android/_/android/platform/frameworks/av/+/642fc1d0ef969c29a8882895d992b008b3bb7dd1 >> > -- > -- > 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-99Lt_g9A_vA0jrY4fb-ACJH-sWWQZJwEny_AmCSwPjtHmjQ%40mail.gmail.com > <https://groups.google.com/d/msgid/android-building/CAB-99Lt_g9A_vA0jrY4fb-ACJH-sWWQZJwEny_AmCSwPjtHmjQ%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- -- 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/CAJgzZoqc%3DwDke03BNQ4%3Dkf6Y--UmnFMFjx7hFiBx1h0ycSDSTg%40mail.gmail.com.
