* [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing
@ 2021-08-06 3:21 zhanglianjie
2021-08-06 4:40 ` Joerg Vehlow
2021-08-09 13:29 ` Cyril Hrubis
0 siblings, 2 replies; 5+ messages in thread
From: zhanglianjie @ 2021-08-06 3:21 UTC (permalink / raw)
To: ltp
Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
index 0c39eb89f..f006e4893 100644
--- a/lib/tst_tmpdir.c
+++ b/lib/tst_tmpdir.c
@@ -108,12 +108,18 @@ int tst_tmpdir_created(void)
char *tst_get_tmpdir(void)
{
+ char *ret = NULL;
+
if (TESTDIR == NULL) {
tst_brkm(TBROK, NULL, "you must call tst_tmpdir() first");
return NULL;
}
- return strdup(TESTDIR);
+ ret = strdup(TESTDIR);
+ if (!ret)
+ tst_brkm(TBROK, NULL, "strdup() failed");
+
+ return ret;
}
const char *tst_get_startwd(void)
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing
2021-08-06 3:21 [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing zhanglianjie
@ 2021-08-06 4:40 ` Joerg Vehlow
2021-08-06 5:42 ` zhanglianjie
2021-08-09 13:24 ` Cyril Hrubis
2021-08-09 13:29 ` Cyril Hrubis
1 sibling, 2 replies; 5+ messages in thread
From: Joerg Vehlow @ 2021-08-06 4:40 UTC (permalink / raw)
To: ltp
Hi
On 8/6/2021 5:21 AM, zhanglianjie wrote:
> Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
>
> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
> index 0c39eb89f..f006e4893 100644
> --- a/lib/tst_tmpdir.c
> +++ b/lib/tst_tmpdir.c
> @@ -108,12 +108,18 @@ int tst_tmpdir_created(void)
>
> char *tst_get_tmpdir(void)
> {
> + char *ret = NULL;
> +
> if (TESTDIR == NULL) {
> tst_brkm(TBROK, NULL, "you must call tst_tmpdir() first");
> return NULL;
> }
>
> - return strdup(TESTDIR);
> + ret = strdup(TESTDIR);
Is a failing strdup here really a thing? The only reason strdup should
be able to fail is with ENOMEM.
The only way tst_brkm will work, if strdup fails here is, if TESTDIR is
an extremely huge string (the NULL case is already handled above).
> + if (!ret)
> + tst_brkm(TBROK, NULL, "strdup() failed");
> +
> + return ret;
> }
>
> const char *tst_get_startwd(void)
> --
> 2.20.1
>
>
Joerg
^ permalink raw reply [flat|nested] 5+ messages in thread* [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing
2021-08-06 4:40 ` Joerg Vehlow
@ 2021-08-06 5:42 ` zhanglianjie
2021-08-09 13:24 ` Cyril Hrubis
1 sibling, 0 replies; 5+ messages in thread
From: zhanglianjie @ 2021-08-06 5:42 UTC (permalink / raw)
To: ltp
Hi,
> On 8/6/2021 5:21 AM, zhanglianjie wrote:
>> Signed-off-by: zhanglianjie <zhanglianjie@uniontech.com>
>>
>> diff --git a/lib/tst_tmpdir.c b/lib/tst_tmpdir.c
>> index 0c39eb89f..f006e4893 100644
>> --- a/lib/tst_tmpdir.c
>> +++ b/lib/tst_tmpdir.c
>> @@ -108,12 +108,18 @@ int tst_tmpdir_created(void)
>>
>> ? char *tst_get_tmpdir(void)
>> ? {
>> +??? char *ret = NULL;
>> +
>> ????? if (TESTDIR == NULL) {
>> ????????? tst_brkm(TBROK, NULL, "you must call tst_tmpdir() first");
>> ????????? return NULL;
>> ????? }
>>
>> -??? return strdup(TESTDIR);
>> +??? ret = strdup(TESTDIR);
> Is a failing strdup here really a thing? The only reason strdup should
> be able to fail is with ENOMEM.
> The only way tst_brkm will work, if strdup fails here is, if TESTDIR is
> an extremely huge string (the NULL case is already handled above).
If don?t consider a very huge string, you don?t need to modify it here.
This is a habitual judgment.
Thank you for your review.
>> +??? if (!ret)
>> +??????? tst_brkm(TBROK, NULL, "strdup() failed");
>> +
>> +??? return ret;
>> ? }
>>
>> ? const char *tst_get_startwd(void)
>> --
>> 2.20.1
>>
>>
> Joerg
>
--
Regards,
Zhang Lianjie
^ permalink raw reply [flat|nested] 5+ messages in thread* [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing
2021-08-06 4:40 ` Joerg Vehlow
2021-08-06 5:42 ` zhanglianjie
@ 2021-08-09 13:24 ` Cyril Hrubis
1 sibling, 0 replies; 5+ messages in thread
From: Cyril Hrubis @ 2021-08-09 13:24 UTC (permalink / raw)
To: ltp
Hi!
> > - return strdup(TESTDIR);
> > + ret = strdup(TESTDIR);
> Is a failing strdup here really a thing? The only reason strdup should
> be able to fail is with ENOMEM.
> The only way tst_brkm will work, if strdup fails here is, if TESTDIR is
> an extremely huge string (the NULL case is already handled above).
It's unlikely, but it may happen if:
* Someone runs memeater on the baground along with LTP
* And kernel is set not to overcommit
And in a case of the test lirary I would rather see it written so that
we check everything, even the unlikely errors.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 5+ messages in thread
* [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing
2021-08-06 3:21 [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing zhanglianjie
2021-08-06 4:40 ` Joerg Vehlow
@ 2021-08-09 13:29 ` Cyril Hrubis
1 sibling, 0 replies; 5+ messages in thread
From: Cyril Hrubis @ 2021-08-09 13:29 UTC (permalink / raw)
To: ltp
Hi!
Pushed, thanks.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-08-09 13:29 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-06 3:21 [LTP] [PATCH] lib/tst_tmpdir: tst_get_tmpdir() add error handing zhanglianjie
2021-08-06 4:40 ` Joerg Vehlow
2021-08-06 5:42 ` zhanglianjie
2021-08-09 13:24 ` Cyril Hrubis
2021-08-09 13:29 ` Cyril Hrubis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.