Re: Re*: [PATCH] change contract between system_path and it's callers

Alex Kuleshov [off-list ref] writes:

quoted

One thing to note is that this illustration does not consider memory
pointed at by the "system_wide" variable here (from attr.c)

        static const char *git_etc_gitattributes(void)
        {
                static const char *system_wide;
                if (!system_wide)
                        system_wide = system_path(ETC_GITATTRIBUTES);
                return system_wide;
        }

at the point of process exit as a "leak".

But why? We allocated memory to "system_wide" with system_path, next git
will exit somewhere with die, but system_wide didn't free... Or i'm
wrong here too?

It is in the same league as "static const char *git_dir" and friends
that appear in the file-scope-static of environment.c.  Keeping small
things around to be cleaned up by exit() is not a crime.

Hello Junio,

I returned static to git_etc_gitattributes return value, but now i
can't understand where to free 'system_wide'. As i understand
correctly attr.c has following API for querying attributes:

git_attr

git_check_attr

And as i see in code git_attr doesn't use git_attr_check, so now we
have only git_check_attr and git_all_stars functions which are through
prepare_attr_stack and bootstrap_attr_stack in it uses path which
returned
 by system_path. So you're right if i free etc_attributes like this
and git_etc_gitattributes will return static value we'll have access
to freed memory if it will called again. But we have no access to
etc_attributes outside bootstrap_attr_stack so where will be best
place to free it?

Thank you

2014-11-25 2:50 GMT+06:00 Junio C Hamano [off-list ref]:

Alex Kuleshov [off-list ref] writes:

quoted

quoted

One thing to note is that this illustration does not consider memory
pointed at by the "system_wide" variable here (from attr.c)

        static const char *git_etc_gitattributes(void)
        {
                static const char *system_wide;
                if (!system_wide)
                        system_wide = system_path(ETC_GITATTRIBUTES);
                return system_wide;
        }

at the point of process exit as a "leak".

But why? We allocated memory to "system_wide" with system_path, next git
will exit somewhere with die, but system_wide didn't free... Or i'm
wrong here too?

It is in the same league as "static const char *git_dir" and friends
that appear in the file-scope-static of environment.c.  Keeping small
things around to be cleaned up by exit() is not a crime.

-- 
_________________________
0xAX

But if we still static const char* for git_etc_gitattributes and will
not free it in bootstrap_attr_stack there will no memory leak, just
tested it, so i'll remove free for it.

2014-11-25 12:45 GMT+06:00 Alexander Kuleshov [off-list ref]:

Hello Junio,

I returned static to git_etc_gitattributes return value, but now i
can't understand where to free 'system_wide'. As i understand
correctly attr.c has following API for querying attributes:

git_attr

git_check_attr

And as i see in code git_attr doesn't use git_attr_check, so now we
have only git_check_attr and git_all_stars functions which are through
prepare_attr_stack and bootstrap_attr_stack in it uses path which
returned
 by system_path. So you're right if i free etc_attributes like this
and git_etc_gitattributes will return static value we'll have access
to freed memory if it will called again. But we have no access to
etc_attributes outside bootstrap_attr_stack so where will be best
place to free it?

Thank you

2014-11-25 2:50 GMT+06:00 Junio C Hamano [off-list ref]:

quoted

Alex Kuleshov [off-list ref] writes:

quoted

quoted

One thing to note is that this illustration does not consider memory
pointed at by the "system_wide" variable here (from attr.c)

        static const char *git_etc_gitattributes(void)
        {
                static const char *system_wide;
                if (!system_wide)
                        system_wide = system_path(ETC_GITATTRIBUTES);
                return system_wide;
        }

at the point of process exit as a "leak".

But why? We allocated memory to "system_wide" with system_path, next git
will exit somewhere with die, but system_wide didn't free... Or i'm
wrong here too?

It is in the same league as "static const char *git_dir" and friends
that appear in the file-scope-static of environment.c.  Keeping small
things around to be cleaned up by exit() is not a crime.

--
_________________________
0xAX

-- 
_________________________
0xAX

Hello Junio,

I'm working on this patch and builtin/config.c, i have a little question:

Look, here - https://github.com/git/git/blob/master/builtin/config.c#L509
and in the next if clauses we get allocated path, but if i test it
with given config file with git config -f....
(https://github.com/git/git/blob/master/builtin/config.c#L513) i have
prefix NULL and next if clause doesn't execute, so if i'll put
free(given_config_source.file) after config action i'll get error
because given_config_source.file wasn't allocated. How to be with
this?

Thank you.

2014-11-25 13:04 GMT+06:00 Alexander Kuleshov [off-list ref]:

But if we still static const char* for git_etc_gitattributes and will
not free it in bootstrap_attr_stack there will no memory leak, just
tested it, so i'll remove free for it.

2014-11-25 12:45 GMT+06:00 Alexander Kuleshov [off-list ref]:

quoted

Hello Junio,

I returned static to git_etc_gitattributes return value, but now i
can't understand where to free 'system_wide'. As i understand
correctly attr.c has following API for querying attributes:

git_attr

git_check_attr

And as i see in code git_attr doesn't use git_attr_check, so now we
have only git_check_attr and git_all_stars functions which are through
prepare_attr_stack and bootstrap_attr_stack in it uses path which
returned
 by system_path. So you're right if i free etc_attributes like this
and git_etc_gitattributes will return static value we'll have access
to freed memory if it will called again. But we have no access to
etc_attributes outside bootstrap_attr_stack so where will be best
place to free it?

Thank you

2014-11-25 2:50 GMT+06:00 Junio C Hamano [off-list ref]:

quoted

Alex Kuleshov [off-list ref] writes:

quoted

quoted

One thing to note is that this illustration does not consider memory
pointed at by the "system_wide" variable here (from attr.c)

        static const char *git_etc_gitattributes(void)
        {
                static const char *system_wide;
                if (!system_wide)
                        system_wide = system_path(ETC_GITATTRIBUTES);
                return system_wide;
        }

at the point of process exit as a "leak".

But why? We allocated memory to "system_wide" with system_path, next git
will exit somewhere with die, but system_wide didn't free... Or i'm
wrong here too?

It is in the same league as "static const char *git_dir" and friends
that appear in the file-scope-static of environment.c.  Keeping small
things around to be cleaned up by exit() is not a crime.

--
_________________________
0xAX

--
_________________________
0xAX

-- 
_________________________
0xAX