Re: [PATCH] Split #2: parser/reader separation
Hi, Thanks for the review, commit it now. On Sat, May 2, 2009 at 6:15 AM, Vladimir 'phcoder' Serbinenko wrote: > Patch looks fine for me. I think you can commit it > As for lua put the origin clearly and put the original copyright notice > below grub's one. Tomorrow I'll do the same for freebsd64 patch and will > merge it >>> >>> Oh, I think it is quite compact already, have you got any suggestion >>> to reduce its size ? > > The only possibility I've seen is to move grub_parser_execute out of kernel. > However I don't think it's practicable. >> >>> >>> > Also it would be good to write the current parser in "grub>" prompt >>> > (not >>> > necessary for rescue prompt) >>> >>> Good point. >>> >>> -- >>> Bean >>> >>> >>> ___ >>> Grub-devel mailing list >>> Grub-devel@gnu.org >>> http://lists.gnu.org/mailman/listinfo/grub-devel >> >> >> >> -- >> Regards >> Vladimir 'phcoder' Serbinenko > > > > -- > Regards > Vladimir 'phcoder' Serbinenko > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
Patch looks fine for me. I think you can commit it As for lua put the origin clearly and put the original copyright notice below grub's one. Tomorrow I'll do the same for freebsd64 patch and will merge it > Oh, I think it is quite compact already, have you got any suggestion >> to reduce its size ? >> > The only possibility I've seen is to move grub_parser_execute out of kernel. However I don't think it's practicable. > > >> > Also it would be good to write the current parser in "grub>" prompt (not >> > necessary for rescue prompt) >> >> Good point. >> >> -- >> Bean >> >> >> ___ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel >> > > > > -- > Regards > Vladimir 'phcoder' Serbinenko > -- Regards Vladimir 'phcoder' Serbinenko ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
On Mon, Apr 27, 2009 at 7:51 AM, Bean wrote: > On Sun, Apr 26, 2009 at 11:50 PM, Vladimir 'phcoder' Serbinenko > wrote: > > Hello this patch breaks grub-emu. Also some files include grub/rescue.h > and > > this has to be fixed. > > Hi, > > For large patch, I normally add only the necessary changes to make it > compile in i386-pc. As commits are quite frequent, this would minimize > the effort to sync with svn head. I'd fix those small issue before > final commitment. > Ok. However a Changlog would simplify the review because then it would be easy to see which parts are newly written and which is just code moving around > > > Also there is a bug is that in case of syntax error reader continues to > ask > > for more lines. E.g > > grub> if > >>; > > syntax error > >> > > Then it's impossible to exit from this "bug mode". It's not your fault > but > > since you touch this code could you fix this? > > I believe a proper way to solve this is to add an eol character, for > example ctrl-D. Then we have a way to break from console input. But > this fix is not trivial, perhaps it should be in a separate patch. > Ok > > > This patch increases the size of kernel by 504 bytes and the size of > > core.img by 224 bytes. Is there a way to do the same thing in a more > compact > > way? > > Oh, I think it is quite compact already, have you got any suggestion > to reduce its size ? > I'll have a look at it again today or tomorrow > > > Also it would be good to write the current parser in "grub>" prompt (not > > necessary for rescue prompt) > > Good point. > > -- > Bean > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > -- Regards Vladimir 'phcoder' Serbinenko ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
On Sun, Apr 26, 2009 at 11:50 PM, Vladimir 'phcoder' Serbinenko wrote: > Hello this patch breaks grub-emu. Also some files include grub/rescue.h and > this has to be fixed. Hi, For large patch, I normally add only the necessary changes to make it compile in i386-pc. As commits are quite frequent, this would minimize the effort to sync with svn head. I'd fix those small issue before final commitment. > Also there is a bug is that in case of syntax error reader continues to ask > for more lines. E.g > grub> if >>; > syntax error >> > Then it's impossible to exit from this "bug mode". It's not your fault but > since you touch this code could you fix this? I believe a proper way to solve this is to add an eol character, for example ctrl-D. Then we have a way to break from console input. But this fix is not trivial, perhaps it should be in a separate patch. > This patch increases the size of kernel by 504 bytes and the size of > core.img by 224 bytes. Is there a way to do the same thing in a more compact > way? Oh, I think it is quite compact already, have you got any suggestion to reduce its size ? > Also it would be good to write the current parser in "grub>" prompt (not > necessary for rescue prompt) Good point. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
Hello this patch breaks grub-emu. Also some files include grub/rescue.h and this has to be fixed. Also there is a bug is that in case of syntax error reader continues to ask for more lines. E.g grub> if >; syntax error > Then it's impossible to exit from this "bug mode". It's not your fault but since you touch this code could you fix this? This patch increases the size of kernel by 504 bytes and the size of core.img by 224 bytes. Is there a way to do the same thing in a more compact way? Also it would be good to write the current parser in "grub>" prompt (not necessary for rescue prompt) On Sat, Apr 11, 2009 at 6:36 PM, Bean wrote: > Hi, > > This patch moves the parser from normal.mod to script/sh, it also > split rescue mode to rescue_reader and rescue_parser. > > dynamic command support and auto fs loader is moved to standalone > source file normal/dyncmd.c and normal/autofs.c. > > -- > Bean > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'phcoder' Serbinenko ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
Hello. Could you say how it influences the size of kernel and also write a changlog? I looked at it and it seems to be ok but I'm not sure that I haven't missed few files between moved functions. I haven't tested the patch yet On Wed, Apr 22, 2009 at 7:47 AM, Bean wrote: > Hi, > > ping ? > > On Sun, Apr 12, 2009 at 12:36 AM, Bean wrote: > > Hi, > > > > This patch moves the parser from normal.mod to script/sh, it also > > split rescue mode to rescue_reader and rescue_parser. > > > > dynamic command support and auto fs loader is moved to standalone > > source file normal/dyncmd.c and normal/autofs.c. > > > > -- > > Bean > > > > > > -- > Bean > > > ___ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Split #2: parser/reader separation
Hi, ping ? On Sun, Apr 12, 2009 at 12:36 AM, Bean wrote: > Hi, > > This patch moves the parser from normal.mod to script/sh, it also > split rescue mode to rescue_reader and rescue_parser. > > dynamic command support and auto fs loader is moved to standalone > source file normal/dyncmd.c and normal/autofs.c. > > -- > Bean > -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
