search

boutell / cgic

cgic: an ANSI C library for CGI Programming
Other
249 stars 96 forks source link

Prevent leaving temp files around after close/crash #7

Closed tenox7 closed 5 years ago

tenox7 commented 6 years ago

unlink() temporary file just after open() so that they will be already deleted in case of disconnect, stop or crash. The change requires using file descriptors and dup().

tenox7 commented 6 years ago

Tom

Are you referring to this:

            char tfileName[1024];

Thanks, Antoni

From: Tom Boutell notifications@github.com Sent: Saturday, August 4, 2018 6:41 AM To: boutell/cgic cgic@noreply.github.com Cc: Antoni Sawicki as@tenoware.com; Author author@noreply.github.com Subject: Re: [boutell/cgic] Prevent leaving temp files around after close/crash (#7)

@boutell requested changes on this pull request.

You can't return a reference to memory that was allocated on the stack in a function that has returned. Sooner or later that will crash/cause instability.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/boutell/cgic/pull/7#pullrequestreview-143382843, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AHxiMntaQaGZjj2uwoVz0PbZzR6ReosPks5uNaRfgaJpZM4VPx6J.

boutell commented 6 years ago

Yes. Any data that will survive past the return of the function must be allocated on the heap, not the stack. C does not have automatic memory management.

On Mon, Aug 6, 2018, 11:05 PM Antoni Sawicki notifications@github.com wrote:

Tom

Are you referring to this:

char tfileName[1024];

Thanks, Antoni

From: Tom Boutell notifications@github.com Sent: Saturday, August 4, 2018 6:41 AM To: boutell/cgic cgic@noreply.github.com Cc: Antoni Sawicki as@tenoware.com; Author author@noreply.github.com Subject: Re: [boutell/cgic] Prevent leaving temp files around after close/crash (#7)

@boutell requested changes on this pull request.

You can't return a reference to memory that was allocated on the stack in a function that has returned. Sooner or later that will crash/cause instability.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub< https://github.com/boutell/cgic/pull/7#pullrequestreview-143382843>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AHxiMntaQaGZjj2uwoVz0PbZzR6ReosPks5uNaRfgaJpZM4VPx6J>.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-410918325, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fVTdTA4aTXpBtrU3xSTKpqiKulpuks5uOQQHgaJpZM4VPx6J .

tenox7 commented 6 years ago

This function neither returns this value or expects it to survive past completion. tfilename is only used internally for mkstemp, chmod, fopen and unlink. The function sets a pointer to FILE **tFile which is used externally. Even if tfilename was allocated in heap I'd be freeing it at tend of the function.

On 08/07/2018 03:41 AM, Tom Boutell wrote:

Yes. Any data that will survive past the return of the function must be allocated on the heap, not the stack. C does not have automatic memory management.

On Mon, Aug 6, 2018, 11:05 PM Antoni Sawicki notifications@github.com wrote:

Tom

Are you referring to this:

char tfileName[1024];

Thanks, Antoni

From: Tom Boutell notifications@github.com Sent: Saturday, August 4, 2018 6:41 AM To: boutell/cgic cgic@noreply.github.com Cc: Antoni Sawicki as@tenoware.com; Author author@noreply.github.com Subject: Re: [boutell/cgic] Prevent leaving temp files around after close/crash (#7)

@boutell requested changes on this pull request.

You can't return a reference to memory that was allocated on the stack in a function that has returned. Sooner or later that will crash/cause instability.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub< https://github.com/boutell/cgic/pull/7#pullrequestreview-143382843>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AHxiMntaQaGZjj2uwoVz0PbZzR6ReosPks5uNaRfgaJpZM4VPx6J>.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-410918325, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fVTdTA4aTXpBtrU3xSTKpqiKulpuks5uOQQHgaJpZM4VPx6J .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-411014276, or mute the thread https://github.com/notifications/unsubscribe-auth/AHxiMvbk-HSFPyxgXGyTcZ3N7HKwOr7Gks5uOW7tgaJpZM4VPx6J.

boutell commented 6 years ago

OK yes, I misread. You're delivering the file pointer, not the filename.

On Tue, Aug 7, 2018 at 3:25 PM, Antoni Sawicki notifications@github.com wrote:

This function neither returns this value or expects it to survive past completion. tfilename is only used internally for mkstemp, chmod, fopen and unlink. The function sets a pointer to FILE **tFile which is used externally. Even if tfilename was allocated in heap I'd be freeing it at tend of the function.

On 08/07/2018 03:41 AM, Tom Boutell wrote:

Yes. Any data that will survive past the return of the function must be allocated on the heap, not the stack. C does not have automatic memory management.

On Mon, Aug 6, 2018, 11:05 PM Antoni Sawicki notifications@github.com wrote:

Tom

Are you referring to this:

char tfileName[1024];

Thanks, Antoni

From: Tom Boutell notifications@github.com Sent: Saturday, August 4, 2018 6:41 AM To: boutell/cgic cgic@noreply.github.com Cc: Antoni Sawicki as@tenoware.com; Author < author@noreply.github.com> Subject: Re: [boutell/cgic] Prevent leaving temp files around after close/crash (#7)

@boutell requested changes on this pull request.

You can't return a reference to memory that was allocated on the stack in a function that has returned. Sooner or later that will crash/cause instability.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub< https://github.com/boutell/cgic/pull/7#pullrequestreview-143382843>, or mute the thread< https://github.com/notifications/unsubscribe-auth/ AHxiMntaQaGZjj2uwoVz0PbZzR6ReosPks5uNaRfgaJpZM4VPx6J>.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-410918325, or mute the thread https://github.com/notifications/unsubscribe-auth/ AAB9fVTdTA4aTXpBtrU3xSTKpqiKulpuks5uOQQHgaJpZM4VPx6J .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/boutell/cgic/pull/7#issuecomment-411014276>, or mute the thread https://github.com/notifications/unsubscribe-auth/AHxiMvbk- HSFPyxgXGyTcZ3N7HKwOr7Gks5uOW7tgaJpZM4VPx6J.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-411173015, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fX7n6Ov-GCLsNTq7RerKLs-OR5p4ks5uOenFgaJpZM4VPx6J .

--

THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT P'UNK AVENUE | (215) 755-1330 | punkave.com

boutell commented 6 years ago

I think my confusion was caused by the continued use of the function name getTempFileName. getTempFile would make more sense now.

On Wed, Aug 8, 2018 at 9:25 AM, Tom Boutell tom@punkave.com wrote:

OK yes, I misread. You're delivering the file pointer, not the filename.

On Tue, Aug 7, 2018 at 3:25 PM, Antoni Sawicki notifications@github.com wrote:

This function neither returns this value or expects it to survive past completion. tfilename is only used internally for mkstemp, chmod, fopen and unlink. The function sets a pointer to FILE **tFile which is used externally. Even if tfilename was allocated in heap I'd be freeing it at tend of the function.

On 08/07/2018 03:41 AM, Tom Boutell wrote:

Yes. Any data that will survive past the return of the function must be allocated on the heap, not the stack. C does not have automatic memory management.

On Mon, Aug 6, 2018, 11:05 PM Antoni Sawicki notifications@github.com wrote:

Tom

Are you referring to this:

char tfileName[1024];

Thanks, Antoni

From: Tom Boutell notifications@github.com Sent: Saturday, August 4, 2018 6:41 AM To: boutell/cgic cgic@noreply.github.com Cc: Antoni Sawicki as@tenoware.com; Author < author@noreply.github.com> Subject: Re: [boutell/cgic] Prevent leaving temp files around after close/crash (#7)

@boutell requested changes on this pull request.

You can't return a reference to memory that was allocated on the stack in a function that has returned. Sooner or later that will crash/cause instability.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub< https://github.com/boutell/cgic/pull/7#pullrequestreview-143382843>, or mute the thread< https://github.com/notifications/unsubscribe-auth/AHxiMntaQa GZjj2uwoVz0PbZzR6ReosPks5uNaRfgaJpZM4VPx6J>.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-410918325, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fVTdT A4aTXpBtrU3xSTKpqiKulpuks5uOQQHgaJpZM4VPx6J .

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/boutell/cgic/pull/7#issuecomment-411014276>, or mute the thread https://github.com/notifications/unsubscribe-auth/AHxiMvbk- HSFPyxgXGyTcZ3N7HKwOr7Gks5uOW7tgaJpZM4VPx6J.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-411173015, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fX7n6Ov-GCLsNTq7RerKLs-OR5p4ks5uOenFgaJpZM4VPx6J .

--

THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT P'UNK AVENUE | (215) 755-1330 | punkave.com

--

THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT P'UNK AVENUE | (215) 755-1330 | punkave.com

tenox7 commented 6 years ago

So any chance of getting this merged? Thanks!

boutell commented 6 years ago

I think the function name should be changed to "getTempFile" because it does not yield a name anymore, it yields the actual file. Otherwise no objections.

On Wed, Aug 29, 2018 at 1:34 AM, Antoni Sawicki notifications@github.com wrote:

So any chance of getting this merged? Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/boutell/cgic/pull/7#issuecomment-416829733, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB9fYjMvHIOWRTev_z9RqH2gZwz9XVMks5uVifKgaJpZM4VPx6J .

--

THOMAS BOUTELL, CHIEF SOFTWARE ARCHITECT P'UNK AVENUE | (215) 755-1330 | punkave.com

tenox7 commented 5 years ago

I have created new PR with changed getTempFileName to getTempFile