Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[Juha Sointusalo]

On 3 January 2017 at 17:13, Christian Beer 
wrote:

> So the safe_copy()
> function serves two purposes. It copies a string into a new char array
> to make it modifiable and usable outside of the string context.
>

Putting aside the bigger issues for a moment. The next time you need a copy
of a C string just use strdup(). It's standard and everybody knows it
allocates memory that needs to be free()d.

-Juha
___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[Christian Beer]

Is this an official BOINC policy? I can't find it on
http://boinc.berkeley.edu/trac/wiki/CodingStyle

Why does it need to be stated in the function name? Is there a common
naming scheme for this kind of documentation? Does this apply to other
functions too?

Regards
Christian

On 03.01.2017 17:17, David Anderson wrote:
> The policy in BOINC is to minimize dynamic allocation and thus avoid
> the complexity of pointer ownership.
> We avoid functions that return pointers to allocated memory.
>
> Exceptions to this should be clearly indicated in the function name,
> e.g. read_file_malloc().
>
> On 1/3/2017 7:13 AM, Christian Beer wrote:
>> On 03.01.2017 15:29, McLeod, John wrote:
>>> What about the std::string function c_str?  It returns a
>>> non-modifiable pointer to the null terminated data in the string?
>> This function (std::string::c_str) provides a non-modifiable pointer and
>> the pointer is only valid as long as the string exists. So you can't use
>> the pointer outside of the context of the string. So the safe_copy()
>> function serves two purposes. It copies a string into a new char array
>> to make it modifiable and usable outside of the string context.
>>
>> Another solution would be to only use strings and use c_str() locally
>> when it is needed and not pass around pointers to char arrays.
>>
>> Regards
>> Christian
>>
>> ___
>> boinc_dev mailing list
>> boinc_dev@ssl.berkeley.edu
>> http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
>> To unsubscribe, visit the above URL and
>> (near bottom of page) enter your email address.
>
> ___
> boinc_dev mailing list
> boinc_dev@ssl.berkeley.edu
> http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
> To unsubscribe, visit the above URL and
> (near bottom of page) enter your email address.


___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[Christian Beer]

The scheduler can also be used as an fcgi executable in which case it
runs indefinitely like the file_upload_handler which would expose the
memory leak.

Regards
Christian

On 03.01.2017 17:18, David Anderson wrote:
> It's OK in this case because it's in the scheduler, which runs a
> limited number of times then exits.
> It's not OK in the client.
>
> On 1/3/2017 7:34 AM, Christian Beer wrote:
>> As for memory leaks occurring in BOINC I'd like to quote
>> sched/sched_version.cpp line 515:
>>> // This is a memory leak, but that's OK
>>
>
> ___
> boinc_dev mailing list
> boinc_dev@ssl.berkeley.edu
> http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
> To unsubscribe, visit the above URL and
> (near bottom of page) enter your email address.


___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[David Anderson]

It's OK in this case because it's in the scheduler, which runs a limited number of 
times then exits.

It's not OK in the client.

On 1/3/2017 7:34 AM, Christian Beer wrote:

As for memory leaks occurring in BOINC I'd like to quote
sched/sched_version.cpp line 515:

// This is a memory leak, but that's OK




___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[David Anderson]

The policy in BOINC is to minimize dynamic allocation and thus avoid
the complexity of pointer ownership.
We avoid functions that return pointers to allocated memory.

Exceptions to this should be clearly indicated in the function name,
e.g. read_file_malloc().

On 1/3/2017 7:13 AM, Christian Beer wrote:

On 03.01.2017 15:29, McLeod, John wrote:

What about the std::string function c_str?  It returns a non-modifiable pointer 
to the null terminated data in the string?

This function (std::string::c_str) provides a non-modifiable pointer and
the pointer is only valid as long as the string exists. So you can't use
the pointer outside of the context of the string. So the safe_copy()
function serves two purposes. It copies a string into a new char array
to make it modifiable and usable outside of the string context.

Another solution would be to only use strings and use c_str() locally
when it is needed and not pass around pointers to char arrays.

Regards
Christian

___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.


___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[Christian Beer]

I agree. But this would also be the more expensive solution (in terms of
man-hours). And also given the fact that we don't have a test framework
to make sure that switching to std::string does not break anything adds
a lot of risk. Whereas my little function has a defined behavior and a
small risk.

As for memory leaks occurring in BOINC I'd like to quote
sched/sched_version.cpp line 515:
> // This is a memory leak, but that's OK

Regards
Christian

On 03.01.2017 16:15, McLeod, John wrote:
> The second would be far better.  It would tend to stop memory leaks from 
> occurring.
>
> -Original Message-
> From: Christian Beer [mailto:christian.b...@aei.mpg.de] 
> Sent: Tuesday, January 3, 2017 10:13 AM
> To: McLeod, John ; boinc_dev@ssl.berkeley.edu
> Subject: Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not 
> used, generated compi...
>
> On 03.01.2017 15:29, McLeod, John wrote:
>> What about the std::string function c_str?  It returns a non-modifiable 
>> pointer to the null terminated data in the string?
> This function (std::string::c_str) provides a non-modifiable pointer and
> the pointer is only valid as long as the string exists. So you can't use
> the pointer outside of the context of the string. So the safe_copy()
> function serves two purposes. It copies a string into a new char array
> to make it modifiable and usable outside of the string context.
>
> Another solution would be to only use strings and use c_str() locally
> when it is needed and not pass around pointers to char arrays.
>
> Regards
> Christian
>

___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[McLeod, John]

The second would be far better.  It would tend to stop memory leaks from 
occurring.

-Original Message-
From: Christian Beer [mailto:christian.b...@aei.mpg.de] 
Sent: Tuesday, January 3, 2017 10:13 AM
To: McLeod, John ; boinc_dev@ssl.berkeley.edu
Subject: Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not 
used, generated compi...

On 03.01.2017 15:29, McLeod, John wrote:
> What about the std::string function c_str?  It returns a non-modifiable 
> pointer to the null terminated data in the string?

This function (std::string::c_str) provides a non-modifiable pointer and
the pointer is only valid as long as the string exists. So you can't use
the pointer outside of the context of the string. So the safe_copy()
function serves two purposes. It copies a string into a new char array
to make it modifiable and usable outside of the string context.

Another solution would be to only use strings and use c_str() locally
when it is needed and not pass around pointers to char arrays.

Regards
Christian

___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[Christian Beer]

On 03.01.2017 15:29, McLeod, John wrote:
> What about the std::string function c_str?  It returns a non-modifiable 
> pointer to the null terminated data in the string?

This function (std::string::c_str) provides a non-modifiable pointer and
the pointer is only valid as long as the string exists. So you can't use
the pointer outside of the context of the string. So the safe_copy()
function serves two purposes. It copies a string into a new char array
to make it modifiable and usable outside of the string context.

Another solution would be to only use strings and use c_str() locally
when it is needed and not pass around pointers to char arrays.

Regards
Christian

___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.

Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not used, generated compi...

[McLeod, John]

What about the std::string function c_str?  It returns a non-modifiable pointer 
to the null terminated data in the string?

-Original Message-
From: boinc_dev [mailto:boinc_dev-boun...@ssl.berkeley.edu] On Behalf Of 
Christian Beer
Sent: Tuesday, December 20, 2016 7:22 AM
To: boinc_dev@ssl.berkeley.edu
Subject: Re: [boinc_dev] [BOINC/boinc] 341001: lib: remove safe_copy(); not 
used, generated compi...

I don't care about the function. I'll just put it back in when I need it
again. What I'm more offended by is the reason that is stated in the
commit message. In my opinion that is not a valid reason to remove
contributed code without discussion or the chance for the contributor to
explain purpose or rectify errors.

- This function represents a nice way to convert std::string to char*
and may become handy in the future. I put it in BOINC because it is not
project-specific and may be something projects find helpful to be in the
BOINC library.
- Indicating implementation details in function names is not useful in
this context in my opinion. From the name alone I wouldn't think that I
need to free the memory myself. Such kind of post conditions are usually
expressed in a descriptive comment above the function. Usually in a
format that can be understood by a documentation tool like doxygen where
the post condition would then be part of the developer documentation
(the place where I would look for such post conditions).
- With a proper post condition that states "You are the owner of the
allocated memory of this function, clean this up after you used it" it
should be clear who the owner is.

Regards
Christian

On 19.12.2016 22:02, David Anderson wrote:
> I'll put it back if you feel that strongly.
> I removed it because:
> - it's not used anywhere (if needed for project-specific code, can
> define it there)
> - the name should be something like safe_copy_alloc() to indicate that
> it allocates memory
> - I try to avoid memory allocation in BOINC because pointer ownership
> is gnarly.
> .
> - David
>
> On 12/19/2016 5:18 AM, Christian Beer wrote:
>> Hi,
>>
>> am I the only one who has a problem with this commit?
>>
>> The BOINC Governance document states that actions of Committers are
>> reviewed after they committed something. Here we are. I want to complain
>> about this commit by a Committer of the BOJNC project.
>>
>> I added this function for a reason and yes it was intended to be used in
>> the server which means Linux only. So if it produces compile errors on
>> Windows we do have better options than removing it from the library. The
>> fact that it is currently not used in the BOINC code should not be the
>> sole reason to throw it away. hat if a project uses this function in
>> project specific daemons?
>>
>> Regards
>> Christian
>>
>> On 19.12.2016 10:17, GitHub wrote:
>>> Branch: refs/heads/master
>>>Home:   https://github.com/BOINC/boinc
>>>Commit: 3410015b00f13f53e8284e668b503b31c20814e4
>>>   
>>> https://github.com/BOINC/boinc/commit/3410015b00f13f53e8284e668b503b31c20814e4
>>>Author: David Anderson 
>>>Date:   2016-12-19 (Mon, 19 Dec 2016)
>>>
>>>Changed paths:
>>>  M lib/str_util.cpp
>>>  M lib/str_util.h
>>>
>>>Log Message:
>>>---
>>>lib: remove safe_copy(); not used, generated compile warnings on Win
>>
>> ___
>> boinc_dev mailing list
>> boinc_dev@ssl.berkeley.edu
>> http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
>> To unsubscribe, visit the above URL and
>> (near bottom of page) enter your email address.
>
> ___
> boinc_dev mailing list
> boinc_dev@ssl.berkeley.edu
> http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
> To unsubscribe, visit the above URL and
> (near bottom of page) enter your email address.


___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.
___
boinc_dev mailing list
boinc_dev@ssl.berkeley.edu
http://lists.ssl.berkeley.edu/mailman/listinfo/boinc_dev
To unsubscribe, visit the above URL and
(near bottom of page) enter your email address.