| Message ID | 20171024105822.31118-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Lua: make table2image() and image2table() symmetric | expand |
Hi Christian, On 24/10/2017 12:58, Christian Storm wrote: > Make table2image() transport the values back to the C domain > that image2table() has transported to the Lua domain beforehand. > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > corelib/lua_interface.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > index dcfcdf3..97636e3 100644 > --- a/corelib/lua_interface.c > +++ b/corelib/lua_interface.c > @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, > { > if (!strcmp(key, "compressed")) > img->compressed = (bool)val; > - if (!strcmp(key, "installed-directly")) > + if (!strcmp(key, "installed_directly")) There is some kind of misalignment. I see that we have currently a bug, because push() and pop() are working on a different varriable name (installed_directly vs installed-directly). However, in the whole project we use installed-directly, and it looks to me straightforward to align the names to this one, and not the opposite. > img->install_directly = (bool)val; > if (!strcmp(key, "install_if_different")) > img->id.install_if_different = (bool)val; > if (!strcmp(key, "encrypted")) > img->is_encrypted = (bool)val; > + if (!strcmp(key, "partition")) > + img->is_partitioner = (bool)val; > + if (!strcmp(key, "script")) > + img->is_script = (bool)val; > } > > static void lua_number_to_img(struct img_type *img, const char *key, > @@ -253,6 +257,10 @@ static void lua_number_to_img(struct img_type *img, const char *key, > { > if (!strcmp(key, "offset")) > img->seek = (unsigned long long)val; > + if (!strcmp(key, "size")) > + img->size = (long long)val; > + if (!strcmp(key, "checksum")) > + img->checksum = (unsigned int)val; > > } > > Best regards, Stefano
Hi Stefano, > > Make table2image() transport the values back to the C domain > > that image2table() has transported to the Lua domain beforehand. > > > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > > --- > > corelib/lua_interface.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > > index dcfcdf3..97636e3 100644 > > --- a/corelib/lua_interface.c > > +++ b/corelib/lua_interface.c > > @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, > > { > > if (!strcmp(key, "compressed")) > > img->compressed = (bool)val; > > - if (!strcmp(key, "installed-directly")) > > + if (!strcmp(key, "installed_directly")) > > There is some kind of misalignment. I see that we have currently a bug, > because push() and pop() are working on a different varriable name > (installed_directly vs installed-directly). However, in the whole > project we use installed-directly, and it looks to me straightforward to > align the names to this one, and not the opposite. Hm, in Lua, a dash '-' isn't allowed to be part of an identifier, so when settling on 'installed-directly', then it has to be "quoted" everywhere, i.e., image["installed-directly"] has to be used instead of the more "natural" image.installed_directly Same is true for 'install_if_different' I guess. While I think it's more Lua-idiomatic to use underscores, I don't have a strong opinion on this, but still wanted to mention it.. So, what do you want to see? I'll prepare patches for it then... Kind regards, Christian
Hi Christian, On 02/11/2017 09:46, Christian Storm wrote: > Hi Stefano, > >>> Make table2image() transport the values back to the C domain >>> that image2table() has transported to the Lua domain beforehand. >>> >>> Signed-off-by: Christian Storm <christian.storm@siemens.com> >>> --- >>> corelib/lua_interface.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c >>> index dcfcdf3..97636e3 100644 >>> --- a/corelib/lua_interface.c >>> +++ b/corelib/lua_interface.c >>> @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, >>> { >>> if (!strcmp(key, "compressed")) >>> img->compressed = (bool)val; >>> - if (!strcmp(key, "installed-directly")) >>> + if (!strcmp(key, "installed_directly")) >> >> There is some kind of misalignment. I see that we have currently a bug, >> because push() and pop() are working on a different varriable name >> (installed_directly vs installed-directly). However, in the whole >> project we use installed-directly, and it looks to me straightforward to >> align the names to this one, and not the opposite. > > Hm, in Lua, a dash '-' isn't allowed to be part of an identifier, Ouch...I have not considered this. > so when settling on 'installed-directly', then it has to be "quoted" > everywhere, i.e., > image["installed-directly"] > has to be used instead of the more "natural" > image.installed_directly > > Same is true for 'install_if_different' I guess. > > While I think it's more Lua-idiomatic to use underscores, I don't have a > strong opinion on this, but still wanted to mention it.. > > So, what do you want to see? I'll prepare patches for it then... We have currently no documentation about interface to LUA because it is supposed that names do not change. If we introduce a discrepancy, we should very well document. Best regards, Stefano
Hi Stefano, > >>> Make table2image() transport the values back to the C domain > >>> that image2table() has transported to the Lua domain beforehand. > >>> > >>> Signed-off-by: Christian Storm <christian.storm@siemens.com> > >>> --- > >>> corelib/lua_interface.c | 10 +++++++++- > >>> 1 file changed, 9 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > >>> index dcfcdf3..97636e3 100644 > >>> --- a/corelib/lua_interface.c > >>> +++ b/corelib/lua_interface.c > >>> @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, > >>> { > >>> if (!strcmp(key, "compressed")) > >>> img->compressed = (bool)val; > >>> - if (!strcmp(key, "installed-directly")) > >>> + if (!strcmp(key, "installed_directly")) > >> > >> There is some kind of misalignment. I see that we have currently a bug, > >> because push() and pop() are working on a different varriable name > >> (installed_directly vs installed-directly). However, in the whole > >> project we use installed-directly, and it looks to me straightforward to > >> align the names to this one, and not the opposite. > > > > Hm, in Lua, a dash '-' isn't allowed to be part of an identifier, > > Ouch...I have not considered this. > > > so when settling on 'installed-directly', then it has to be "quoted" > > everywhere, i.e., > > image["installed-directly"] > > has to be used instead of the more "natural" > > image.installed_directly > > > > Same is true for 'install_if_different' I guess. > > > > While I think it's more Lua-idiomatic to use underscores, I don't have a > > strong opinion on this, but still wanted to mention it.. > > > > So, what do you want to see? I'll prepare patches for it then... > > We have currently no documentation about interface to LUA because it is > supposed that names do not change. If we introduce a discrepancy, we > should very well document. OK, since I'm in the process of preparing and documenting the Lua first-class-citizen handler feature anyway, I'll add notes on this to the documentation... Kind regards, Christian
Hi Stefano, > > >>> Make table2image() transport the values back to the C domain > > >>> that image2table() has transported to the Lua domain beforehand. > > >>> > > >>> Signed-off-by: Christian Storm <christian.storm@siemens.com> > > >>> --- > > >>> corelib/lua_interface.c | 10 +++++++++- > > >>> 1 file changed, 9 insertions(+), 1 deletion(-) > > >>> > > >>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c > > >>> index dcfcdf3..97636e3 100644 > > >>> --- a/corelib/lua_interface.c > > >>> +++ b/corelib/lua_interface.c > > >>> @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, > > >>> { > > >>> if (!strcmp(key, "compressed")) > > >>> img->compressed = (bool)val; > > >>> - if (!strcmp(key, "installed-directly")) > > >>> + if (!strcmp(key, "installed_directly")) > > >> > > >> There is some kind of misalignment. I see that we have currently a bug, > > >> because push() and pop() are working on a different varriable name > > >> (installed_directly vs installed-directly). However, in the whole > > >> project we use installed-directly, and it looks to me straightforward to > > >> align the names to this one, and not the opposite. > > > > > > Hm, in Lua, a dash '-' isn't allowed to be part of an identifier, > > > > Ouch...I have not considered this. > > > > > so when settling on 'installed-directly', then it has to be "quoted" > > > everywhere, i.e., > > > image["installed-directly"] > > > has to be used instead of the more "natural" > > > image.installed_directly > > > > > > Same is true for 'install_if_different' I guess. > > > > > > While I think it's more Lua-idiomatic to use underscores, I don't have a > > > strong opinion on this, but still wanted to mention it.. > > > > > > So, what do you want to see? I'll prepare patches for it then... > > > > We have currently no documentation about interface to LUA because it is > > supposed that names do not change. If we introduce a discrepancy, we > > should very well document. > > OK, since I'm in the process of preparing and documenting the Lua > first-class-citizen handler feature anyway, I'll add notes on this > to the documentation... In the meantime, I think this patch can be merged, right? Kind regards, Christian
On 02/11/2017 13:26, [ext] Christian Storm wrote: > Hi Stefano, > >>>>>> Make table2image() transport the values back to the C domain >>>>>> that image2table() has transported to the Lua domain beforehand. >>>>>> >>>>>> Signed-off-by: Christian Storm <christian.storm@siemens.com> >>>>>> --- >>>>>> corelib/lua_interface.c | 10 +++++++++- >>>>>> 1 file changed, 9 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c >>>>>> index dcfcdf3..97636e3 100644 >>>>>> --- a/corelib/lua_interface.c >>>>>> +++ b/corelib/lua_interface.c >>>>>> @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, >>>>>> { >>>>>> if (!strcmp(key, "compressed")) >>>>>> img->compressed = (bool)val; >>>>>> - if (!strcmp(key, "installed-directly")) >>>>>> + if (!strcmp(key, "installed_directly")) >>>>> >>>>> There is some kind of misalignment. I see that we have currently a bug, >>>>> because push() and pop() are working on a different varriable name >>>>> (installed_directly vs installed-directly). However, in the whole >>>>> project we use installed-directly, and it looks to me straightforward to >>>>> align the names to this one, and not the opposite. >>>> >>>> Hm, in Lua, a dash '-' isn't allowed to be part of an identifier, >>> >>> Ouch...I have not considered this. >>> >>>> so when settling on 'installed-directly', then it has to be "quoted" >>>> everywhere, i.e., >>>> image["installed-directly"] >>>> has to be used instead of the more "natural" >>>> image.installed_directly >>>> >>>> Same is true for 'install_if_different' I guess. >>>> >>>> While I think it's more Lua-idiomatic to use underscores, I don't have a >>>> strong opinion on this, but still wanted to mention it.. >>>> >>>> So, what do you want to see? I'll prepare patches for it then... >>> >>> We have currently no documentation about interface to LUA because it is >>> supposed that names do not change. If we introduce a discrepancy, we >>> should very well document. >> >> OK, since I'm in the process of preparing and documenting the Lua >> first-class-citizen handler feature anyway, I'll add notes on this >> to the documentation... > > In the meantime, I think this patch can be merged, right? I apply it. Regards, Stefano
diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c index dcfcdf3..97636e3 100644 --- a/corelib/lua_interface.c +++ b/corelib/lua_interface.c @@ -240,12 +240,16 @@ static void lua_bool_to_img(struct img_type *img, const char *key, { if (!strcmp(key, "compressed")) img->compressed = (bool)val; - if (!strcmp(key, "installed-directly")) + if (!strcmp(key, "installed_directly")) img->install_directly = (bool)val; if (!strcmp(key, "install_if_different")) img->id.install_if_different = (bool)val; if (!strcmp(key, "encrypted")) img->is_encrypted = (bool)val; + if (!strcmp(key, "partition")) + img->is_partitioner = (bool)val; + if (!strcmp(key, "script")) + img->is_script = (bool)val; } static void lua_number_to_img(struct img_type *img, const char *key, @@ -253,6 +257,10 @@ static void lua_number_to_img(struct img_type *img, const char *key, { if (!strcmp(key, "offset")) img->seek = (unsigned long long)val; + if (!strcmp(key, "size")) + img->size = (long long)val; + if (!strcmp(key, "checksum")) + img->checksum = (unsigned int)val; }
Make table2image() transport the values back to the C domain that image2table() has transported to the Lua domain beforehand. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- corelib/lua_interface.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)