Hello Holger, On Sat, Dec 2, 2017 at 2:50 PM, Holger Freyther <[email protected]> wrote:
> > > On 30. Nov 2017, at 17:31, Lee Duhem <[email protected]> wrote: > > > > Hi again! > > > > > @@ -2025,7 +2023,7 @@ _gst_to_wide_cstring (OOP stringOOP) > > string = (gst_unicode_string) OOP_TO_OBJ (stringOOP); > > len = oop_num_fields (stringOOP); > > result = (wchar_t *) xmalloc (len + 1); > > - if (sizeof (wchar_t) == 4) > > + if (sizeof (wchar_t) == sizeof(string->chars[0])) > > Okay. string->chars[0] is uint32_t so the above is identical but > the code looks bad in general? > The code implies that when sizeof(string->chars[0]) equals 4, it can memcpy from string->chars to result. This change just wants to make that assumption clear. > > len = oop_num_fields (stringOOP); > result = (wchar_t *) xmalloc (len + 1); > > > '12345678' asUnicodeString basicSize > => 8 > > There is barely any C API using "wchar_t" so it is no surprise > the code might be broken. I think it needs to be: > > result = (wchar_t *) xmalloc ((len + 1) * sizeof(*result)); > > and then maybe size(*result) == sizeof(string->chars[0]) > Yes, we need to xmalloc more spaces for the result. BTW, adding an assertion to make sure sizeof(wchar_t) >= sizeof(string->chars[0]) looks like a good idea to prevent possible overwritten. > > > > > - && (flags & (GST_REBUILD_IMAGE | GST_MAYBE_REBUILD_IMAGE)) == 0 > > + && !rebuild_image_flags > > I am flying right now but what is the GNU coding style? I have > my phases with !flag or flag == 0. I would only change it if there > is either a majority in our code or GNU coding style has a > statement about it. > I replaced that complicated condition with !rebuild_image_flags because we already have that variable, and it seems a good idea for me to simplify it. About !flag or flag == 0, both work for me. I chose !flag because there are already several examples in the same source code file. Regards, lee > > cheers > holger > _______________________________________________ help-smalltalk mailing list [email protected] https://lists.gnu.org/mailman/listinfo/help-smalltalk
