Date: Fri, 30 Nov 2018 02:06:30 +0000 (UTC) From: David Bright <dab@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r341275 - in stable/11: sys/dev/netmap tests/sys/kqueue/libkqueue Message-ID: <201811300206.wAU26U2M028462@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: dab Date: Fri Nov 30 02:06:30 2018 New Revision: 341275 URL: https://svnweb.freebsd.org/changeset/base/341275 Log: MFC r337812,r337814,r337820,r341068: Fix several memory leaks (r337812 & r337814). The libkqueue tests have several places that leak memory by using an idiom like: puts(kevent_to_str(kevp)); Rework to save the pointer returned from kevent_to_str() and then free() it after it has been used. r337812 also fixed a bug in the netmap kevent code. The inclusion of that fix was an oversight that I didn't notice until this MFC. Reference the code review and PR here in the MFC for completeness. r337820 & r341068 were white-space only changes as a follow-up to r337812 & r337814: After r337820, which "corrected" some spaces-instead-of-tab whitespace issues in the libkqueue tests, jmg@ pointed out that these files were originally space-based, not tab-spaced, and so the correction should have been to get rid of the tabs that had been introduced in previous changes, not the spaces. This change does that. This is a whitespace only change; no functional change is intended. PR: 206053 Differential Revision: https://reviews.freebsd.org/D16531 Sponsored by: Dell EMC Isilon Modified: stable/11/sys/dev/netmap/netmap_freebsd.c stable/11/tests/sys/kqueue/libkqueue/common.h stable/11/tests/sys/kqueue/libkqueue/main.c stable/11/tests/sys/kqueue/libkqueue/proc.c stable/11/tests/sys/kqueue/libkqueue/signal.c stable/11/tests/sys/kqueue/libkqueue/timer.c stable/11/tests/sys/kqueue/libkqueue/user.c stable/11/tests/sys/kqueue/libkqueue/vnode.c Directory Properties: stable/11/ (props changed) Modified: stable/11/sys/dev/netmap/netmap_freebsd.c ============================================================================== --- stable/11/sys/dev/netmap/netmap_freebsd.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/sys/dev/netmap/netmap_freebsd.c Fri Nov 30 02:06:30 2018 (r341275) @@ -778,7 +778,7 @@ netmap_kqfilter(struct cdev *dev, struct knote *kn) kn->kn_fop = (ev == EVFILT_WRITE) ? &netmap_wfiltops : &netmap_rfiltops; kn->kn_hook = priv; - knlist_add(&si->si.si_note, kn, 1); + knlist_add(&si->si.si_note, kn, 0); // XXX unlock(priv) ND("register %p %s td %p priv %p kn %p np_nifp %p kn_fp/fpop %s", na, na->ifp->if_xname, curthread, priv, kn, Modified: stable/11/tests/sys/kqueue/libkqueue/common.h ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/common.h Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/common.h Fri Nov 30 02:06:30 2018 (r341275) @@ -43,7 +43,7 @@ extern char *cur_test_id; int vnode_fd; -extern const char * kevent_to_str(struct kevent *); +extern char * kevent_to_str(struct kevent *); struct kevent * kevent_get(int); struct kevent * kevent_get_timeout(int, int); Modified: stable/11/tests/sys/kqueue/libkqueue/main.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/main.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/main.c Fri Nov 30 02:06:30 2018 (r341275) @@ -41,13 +41,16 @@ test_no_kevents(void) int nfds; struct timespec timeo; struct kevent kev; + char *kev_str; puts("confirming that there are no events pending"); memset(&timeo, 0, sizeof(timeo)); nfds = kevent(kqfd, NULL, 0, &kev, 1, &timeo); if (nfds != 0) { puts("\nUnexpected event:"); - puts(kevent_to_str(&kev)); + kev_str = kevent_to_str(&kev); + puts(kev_str); + free(kev_str); errx(1, "%d event(s) pending, but none expected:", nfds); } } @@ -61,12 +64,15 @@ test_no_kevents_quietly(void) int nfds; struct timespec timeo; struct kevent kev; + char *kev_str; memset(&timeo, 0, sizeof(timeo)); nfds = kevent(kqfd, NULL, 0, &kev, 1, &timeo); if (nfds != 0) { puts("\nUnexpected event:"); - puts(kevent_to_str(&kev)); + kev_str = kevent_to_str(&kev); + puts(kev_str); + free(kev_str); errx(1, "%d event(s) pending, but none expected:", nfds); } } @@ -79,7 +85,7 @@ kevent_get(int kqfd) struct kevent *kev; if ((kev = calloc(1, sizeof(*kev))) == NULL) - err(1, "out of memory"); + err(1, "out of memory"); nfds = kevent(kqfd, NULL, 0, kev, 1, NULL); if (nfds < 1) @@ -97,7 +103,7 @@ kevent_get_timeout(int kqfd, int seconds) struct timespec timeout = {seconds, 0}; if ((kev = calloc(1, sizeof(*kev))) == NULL) - err(1, "out of memory"); + err(1, "out of memory"); nfds = kevent(kqfd, NULL, 0, kev, 1, &timeout); if (nfds < 0) { @@ -117,10 +123,10 @@ kevent_fflags_dump(struct kevent *kev) #define KEVFFL_DUMP(attrib) \ if (kev->fflags & attrib) \ - strncat(buf, #attrib" ", 64); + strncat(buf, #attrib" ", 64); if ((buf = calloc(1, 1024)) == NULL) - abort(); + abort(); /* Not every filter has meaningful fflags */ if (kev->filter == EVFILT_PROC) { @@ -154,7 +160,7 @@ kevent_fflags_dump(struct kevent *kev) #endif buf[strlen(buf) - 1] = ')'; } else { - snprintf(buf, 1024, "fflags = %x", kev->fflags); + snprintf(buf, 1024, "fflags = %x", kev->fflags); } return (buf); @@ -167,10 +173,10 @@ kevent_flags_dump(struct kevent *kev) #define KEVFL_DUMP(attrib) \ if (kev->flags & attrib) \ - strncat(buf, #attrib" ", 64); + strncat(buf, #attrib" ", 64); if ((buf = calloc(1, 1024)) == NULL) - abort(); + abort(); snprintf(buf, 1024, "flags = %d (", kev->flags); KEVFL_DUMP(EV_ADD); @@ -193,20 +199,25 @@ kevent_flags_dump(struct kevent *kev) } /* Copied from ../kevent.c kevent_dump() and improved */ -const char * +char * kevent_to_str(struct kevent *kev) { char buf[512]; + char *flags_str = kevent_flags_dump(kev); + char *fflags_str = kevent_fflags_dump(kev); snprintf(&buf[0], sizeof(buf), - "[ident=%d, filter=%d, %s, %s, data=%d, udata=%p]", - (u_int) kev->ident, + "[ident=%ju, filter=%d, %s, %s, data=%jd, udata=%p]", + (uintmax_t) kev->ident, kev->filter, - kevent_flags_dump(kev), - kevent_fflags_dump(kev), - (int) kev->data, + flags_str, + fflags_str, + (uintmax_t)kev->data, kev->udata); + free(flags_str); + free(fflags_str); + return (strdup(buf)); } @@ -219,10 +230,14 @@ kevent_add(int kqfd, struct kevent *kev, intptr_t data, void *udata) { + char *kev_str; + EV_SET(kev, ident, filter, flags, fflags, data, NULL); if (kevent(kqfd, kev, 1, NULL, 0, NULL) < 0) { + kev_str = kevent_to_str(kev); printf("Unable to add the following kevent:\n%s\n", - kevent_to_str(kev)); + kev_str); + free(kev_str); err(1, "kevent(): %s", strerror(errno)); } } @@ -230,6 +245,9 @@ kevent_add(int kqfd, struct kevent *kev, void kevent_cmp(struct kevent *k1, struct kevent *k2) { + char *kev1_str; + char *kev2_str; + /* XXX- Workaround for inconsistent implementation of kevent(2) */ @@ -238,8 +256,12 @@ kevent_cmp(struct kevent *k1, struct kevent *k2) k2->flags |= EV_ADD; #endif if (memcmp(k1, k2, sizeof(*k1)) != 0) { + kev1_str = kevent_to_str(k1); + kev2_str = kevent_to_str(k2); printf("kevent_cmp: mismatch:\n %s !=\n %s\n", - kevent_to_str(k1), kevent_to_str(k2)); + kev1_str, kev2_str); + free(kev1_str); + free(kev2_str); abort(); } } Modified: stable/11/tests/sys/kqueue/libkqueue/proc.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/proc.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/proc.c Fri Nov 30 02:06:30 2018 (r341275) @@ -45,7 +45,7 @@ add_and_delete(void) struct stat s; if (fstat(kqfd, &s) != -1) errx(1, "kqueue inherited across fork! (%s() at %s:%d)", - __func__, __FILE__, __LINE__); + __func__, __FILE__, __LINE__); pause(); exit(2); @@ -172,6 +172,7 @@ proc_track(int sleep_time) int gchild_note = 0; pid_t gchild_pid = -1; int done = 0; + char *kev_str; while (!done) { @@ -182,7 +183,9 @@ proc_track(int sleep_time) if (kevp == NULL) { done = 1; } else { - printf(" -- Received kevent: %s\n", kevent_to_str(kevp)); + kev_str = kevent_to_str(kevp); + printf(" -- Received kevent: %s\n", kev_str); + free(kev_str); if ((kevp->fflags & NOTE_CHILD) && (kevp->fflags & NOTE_EXIT)) { errx(1, "NOTE_CHILD and NOTE_EXIT in same kevent: %s", kevent_to_str(kevp)); Modified: stable/11/tests/sys/kqueue/libkqueue/signal.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/signal.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/signal.c Fri Nov 30 02:06:30 2018 (r341275) @@ -188,12 +188,12 @@ test_kevent_signal_oneshot(void) void test_evfilt_signal() { - kqfd = kqueue(); - test_kevent_signal_add(); - test_kevent_signal_del(); - test_kevent_signal_get(); - test_kevent_signal_disable(); - test_kevent_signal_enable(); - test_kevent_signal_oneshot(); - close(kqfd); + kqfd = kqueue(); + test_kevent_signal_add(); + test_kevent_signal_del(); + test_kevent_signal_get(); + test_kevent_signal_disable(); + test_kevent_signal_enable(); + test_kevent_signal_oneshot(); + close(kqfd); } Modified: stable/11/tests/sys/kqueue/libkqueue/timer.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/timer.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/timer.c Fri Nov 30 02:06:30 2018 (r341275) @@ -34,11 +34,10 @@ int kqfd; static long now(void) { - - struct timeval tv; + struct timeval tv; - gettimeofday(&tv, NULL); - return SEC_TO_US(tv.tv_sec) + tv.tv_usec; + gettimeofday(&tv, NULL); + return SEC_TO_US(tv.tv_sec) + tv.tv_usec; } /* Sleep for a given number of milliseconds. The timeout is assumed to @@ -47,13 +46,12 @@ now(void) void mssleep(int t) { + struct timespec stime = { + .tv_sec = 0, + .tv_nsec = US_TO_NS(MS_TO_US(t)), + }; - struct timespec stime = { - .tv_sec = 0, - .tv_nsec = US_TO_NS(MS_TO_US(t)), - }; - - nanosleep(&stime, NULL); + nanosleep(&stime, NULL); } /* Sleep for a given number of microseconds. The timeout is assumed to @@ -62,13 +60,12 @@ mssleep(int t) void ussleep(int t) { + struct timespec stime = { + .tv_sec = 0, + .tv_nsec = US_TO_NS(t), + }; - struct timespec stime = { - .tv_sec = 0, - .tv_nsec = US_TO_NS(t), - }; - - nanosleep(&stime, NULL); + nanosleep(&stime, NULL); } void @@ -229,16 +226,16 @@ test_update(void) /* First set the timer to 1 second */ EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, - NOTE_USECONDS, SEC_TO_US(1), (void *)1); + NOTE_USECONDS, SEC_TO_US(1), (void *)1); start = now(); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Now reduce the timer to 1 ms */ EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, - NOTE_USECONDS, MS_TO_US(1), (void *)2); + NOTE_USECONDS, MS_TO_US(1), (void *)2); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Wait for the event */ kev.flags |= EV_CLEAR; @@ -253,9 +250,9 @@ test_update(void) */ printf("timer expired after %ld us\n", elapsed); if (elapsed < MS_TO_US(1)) - errx(1, "early timer expiration: %ld us", elapsed); + errx(1, "early timer expiration: %ld us", elapsed); if (elapsed > SEC_TO_US(1)) - errx(1, "late timer expiration: %ld us", elapsed); + errx(1, "late timer expiration: %ld us", elapsed); success(); } @@ -274,9 +271,9 @@ test_update_equal(void) /* First set the timer to 1 ms */ EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, - NOTE_USECONDS, MS_TO_US(1), NULL); + NOTE_USECONDS, MS_TO_US(1), NULL); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Sleep for a significant fraction of the timeout. */ ussleep(600); @@ -284,7 +281,7 @@ test_update_equal(void) /* Now re-add the timer with the same parameters */ start = now(); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Wait for the event */ kev.flags |= EV_CLEAR; @@ -299,7 +296,7 @@ test_update_equal(void) */ printf("timer expired after %ld us\n", elapsed); if (elapsed < MS_TO_US(1)) - errx(1, "early timer expiration: %ld us", elapsed); + errx(1, "early timer expiration: %ld us", elapsed); success(); } @@ -318,9 +315,9 @@ test_update_expired(void) /* Set the timer to 1ms */ EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, - NOTE_USECONDS, MS_TO_US(1), NULL); + NOTE_USECONDS, MS_TO_US(1), NULL); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Wait for 2 ms to give the timer plenty of time to expire. */ mssleep(2); @@ -328,7 +325,7 @@ test_update_expired(void) /* Now re-add the timer */ start = now(); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Wait for the event */ kev.flags |= EV_CLEAR; @@ -343,7 +340,7 @@ test_update_expired(void) */ printf("timer expired after %ld us\n", elapsed); if (elapsed < MS_TO_US(1)) - errx(1, "early timer expiration: %ld us", elapsed); + errx(1, "early timer expiration: %ld us", elapsed); /* Make sure the re-added timer does not fire. In other words, * test that the event received above was the only event from the @@ -370,7 +367,7 @@ test_update_periodic(void) EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD, 0, SEC_TO_MS(1), NULL); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Retrieve the event */ kev.flags = EV_ADD | EV_CLEAR; @@ -385,7 +382,7 @@ test_update_periodic(void) EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD, 0, SEC_TO_MS(2), NULL); start = now(); if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); /* Retrieve the event */ kev.flags = EV_ADD | EV_CLEAR; @@ -399,12 +396,12 @@ test_update_periodic(void) */ printf("timer expired after %ld us\n", elapsed); if (elapsed < MS_TO_US(2)) - errx(1, "early timer expiration: %ld us", elapsed); + errx(1, "early timer expiration: %ld us", elapsed); /* Delete the event */ kev.flags = EV_DELETE; if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + err(1, "%s", test_id); success(); } @@ -432,46 +429,46 @@ test_update_timing(void) * received is from the update and not the original timer add. */ for (sleeptime = MIN_SLEEP, iteration = 1; - sleeptime < MAX_SLEEP; - ++sleeptime, ++iteration) { + sleeptime < MAX_SLEEP; + ++sleeptime, ++iteration) { - /* First set the timer to 1 ms */ - EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, - NOTE_USECONDS, MS_TO_US(1), NULL); - if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + /* First set the timer to 1 ms */ + EV_SET(&kev, vnode_fd, EVFILT_TIMER, EV_ADD | EV_ONESHOT, + NOTE_USECONDS, MS_TO_US(1), NULL); + if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) + err(1, "%s", test_id); - /* Delay; the delay ranges from less than to greater than the - * timer period. - */ - ussleep(sleeptime); + /* Delay; the delay ranges from less than to greater than the + * timer period. + */ + ussleep(sleeptime); - /* Now re-add the timer with the same parameters */ - start = now(); - if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) - err(1, "%s", test_id); + /* Now re-add the timer with the same parameters */ + start = now(); + if (kevent(kqfd, &kev, 1, NULL, 0, NULL) < 0) + err(1, "%s", test_id); - /* Wait for the event */ - kev.flags |= EV_CLEAR; - kev.fflags &= ~NOTE_USECONDS; - kev.data = 1; - kevent_cmp(&kev, kevent_get(kqfd)); - stop = now(); - elapsed = stop - start; + /* Wait for the event */ + kev.flags |= EV_CLEAR; + kev.fflags &= ~NOTE_USECONDS; + kev.data = 1; + kevent_cmp(&kev, kevent_get(kqfd)); + stop = now(); + elapsed = stop - start; - /* Check that the timer expired after at least 1 ms. This - * check is to make sure that the timer re-started and that - * the event is not from the original add of the timer. - */ - if (elapsed < MS_TO_US(1)) - errx(1, "early timer expiration: %ld us", elapsed); + /* Check that the timer expired after at least 1 ms. This + * check is to make sure that the timer re-started and that + * the event is not from the original add of the timer. + */ + if (elapsed < MS_TO_US(1)) + errx(1, "early timer expiration: %ld us", elapsed); - /* Make sure the re-added timer does not fire. In other words, - * test that the event received above was the only event from - * the add and re-add of the timer. - */ - mssleep(2); - test_no_kevents_quietly(); + /* Make sure the re-added timer does not fire. In other words, + * test that the event received above was the only event from + * the add and re-add of the timer. + */ + mssleep(2); + test_no_kevents_quietly(); } success(); @@ -480,17 +477,17 @@ test_update_timing(void) void test_evfilt_timer() { - kqfd = kqueue(); - test_kevent_timer_add(); - test_kevent_timer_del(); - test_kevent_timer_get(); - test_oneshot(); - test_periodic(); - test_update(); - test_update_equal(); - test_update_expired(); - test_update_timing(); - test_update_periodic(); - disable_and_enable(); - close(kqfd); + kqfd = kqueue(); + test_kevent_timer_add(); + test_kevent_timer_del(); + test_kevent_timer_get(); + test_oneshot(); + test_periodic(); + test_update(); + test_update_equal(); + test_update_expired(); + test_update_timing(); + test_update_periodic(); + disable_and_enable(); + close(kqfd); } Modified: stable/11/tests/sys/kqueue/libkqueue/user.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/user.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/user.c Fri Nov 30 02:06:30 2018 (r341275) @@ -117,7 +117,7 @@ oneshot(void) void test_evfilt_user() { - kqfd = kqueue(); + kqfd = kqueue(); add_and_delete(); event_wait(); @@ -125,5 +125,5 @@ test_evfilt_user() oneshot(); /* TODO: try different fflags operations */ - close(kqfd); + close(kqfd); } Modified: stable/11/tests/sys/kqueue/libkqueue/vnode.c ============================================================================== --- stable/11/tests/sys/kqueue/libkqueue/vnode.c Fri Nov 30 01:45:54 2018 (r341274) +++ stable/11/tests/sys/kqueue/libkqueue/vnode.c Fri Nov 30 02:06:30 2018 (r341275) @@ -251,16 +251,16 @@ test_kevent_vnode_dispatch(void) void test_evfilt_vnode() { - kqfd = kqueue(); - test_kevent_vnode_add(); - test_kevent_vnode_del(); - test_kevent_vnode_disable_and_enable(); + kqfd = kqueue(); + test_kevent_vnode_add(); + test_kevent_vnode_del(); + test_kevent_vnode_disable_and_enable(); #if HAVE_EV_DISPATCH - test_kevent_vnode_dispatch(); + test_kevent_vnode_dispatch(); #endif - test_kevent_vnode_note_write(); - test_kevent_vnode_note_attrib(); - test_kevent_vnode_note_rename(); - test_kevent_vnode_note_delete(); - close(kqfd); + test_kevent_vnode_note_write(); + test_kevent_vnode_note_attrib(); + test_kevent_vnode_note_rename(); + test_kevent_vnode_note_delete(); + close(kqfd); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201811300206.wAU26U2M028462>