Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lunatik states management for GSoC 2020 #20

Open
wants to merge 48 commits into
base: master
Choose a base branch
from

Conversation

MatheusNtg
Copy link
Collaborator

No description provided.

@@ -0,0 +1,3 @@
[submodule "deps/lua-memory"]
path = deps/lua-memory
url = https://github.com/MatheusNtg/lua-memory
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should refers to upstream, right? or do you have modifications in your fork?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modifications on my fork.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, you need to make a PR as well.. what's the upstream repo? I think we should fork it on luainkernel and make a PR for it..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upstream repo can be found here. I made a PR to lua-memory repo now

Makefile Outdated
@@ -1,4 +1,5 @@
ccflags-y += -D_LUNATIK -D_KERNEL -I$(src) -D_CONFIG_FULL_PANIC
ccflags-y += -D_LUNATIK -D_KERNEL -I$(src) -D_CONFIG_FULL_PANIC -DLUNATIK_UNUSED \
-I$(src)/lua -I$(src)/deps/lua-memory/src
Copy link
Contributor

@lneto lneto Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
-I$(src)/lua -I$(src)/deps/lua-memory/src
-I$(src)/lua -I$(src)/deps/lua-memory/src

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please review and apply the same to the rest of the code..

Makefile Outdated
lua/lundump.o lua/lvm.o lua/lzio.o lua/lauxlib.o lua/lbaselib.o \
lua/lbitlib.o lua/lcorolib.o lua/ldblib.o lua/lstrlib.o \
lua/ltablib.o lua/lutf8lib.o lua/loslib.o lua/lmathlib.o lua/linit.o \
lua/loadlib.o luautil.o
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why changing all these lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To put all lua related objects in on variable on the Lunatik building

Copy link
Contributor

@lneto lneto Sep 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need change the indentation of this lines to do that.. please, take cara of that..

Makefile Outdated
lua/ltablib.o lua/lutf8lib.o lua/loslib.o lua/lmathlib.o lua/linit.o \
lua/loadlib.o luautil.o

lua_memory-objs = deps/lua-memory/src/lmemlib.o deps/lua-memory/src/lmemmod.o
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lua_memory-objs = deps/lua-memory/src/lmemlib.o deps/lua-memory/src/lmemmod.o
luamemory-path = deps/lua-memory/src
luamemory-objs = $(luamemory-path)/lmemlib.o $(luamemory-path)/lmemmod.o

@@ -0,0 +1,53 @@
# Copyright (c) 2020 Matheus Rodrigues <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you copying this makefile if you have a submodule?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the makefile of lunatik user space module. What are you suggesting is to build the lunatik user space module from the submodule makefile?

lunatik_conf.h Outdated
#ifndef LUNATIK_CONF_H
#define LUNATIK_CONF_H

#define LUNATIK_NAME_MAXSIZE 64 /* Max length of Lua state name */
Copy link
Contributor

@lneto lneto Sep 3, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, use the form:

#define <name>\t(<definition>)

and apply this to all your definitions..

states.c Show resolved Hide resolved
assert(#states == 6)
assert(#states2 == 6)


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/\n//

-- State created from another session
ss2 = session2:newstate's1'
assert(ss2 == nil)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/\n//

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants