Page MenuHomeFreeBSD
Differential D29261

textproc/git-chglog: Update to 0.11.2
ClosedPublic
Actions

Authored by lcook on Mar 14 2021, 5:25 PM.
Tags
None
Referenced Files
F115307856: D29261.id85834.diff
Tue, Apr 22, 11:04 AM
Unknown Object (File)
Sat, Apr 12, 6:31 PM
Unknown Object (File)
Sat, Apr 5, 2:36 PM
Unknown Object (File)
Feb 10 2025, 4:15 PM
Unknown Object (File)
Feb 9 2025, 4:00 PM
Unknown Object (File)
Nov 24 2024, 4:20 PM
Unknown Object (File)
Nov 24 2024, 11:07 AM
Unknown Object (File)
Nov 24 2024, 11:07 AM
View All 51 Files
Subscribers
mat

Details

Reviewers
tcberner
fernape
Commits
rP568563: textproc/git-chglog: Update to 0.11.2
Summary

Changes: https://github.com/git-chglog/git-chglog/compare/v0.10.0...v0.11.2

Test Plan

QA:

  • portlint: OK (looks fine).
  • testport: OK (poudriere: 12.2-RELEASE-p4, amd64/i386).

MFH: No (Prior breaking-changes).

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Revision Contents
Changeset List

PathSize
head/
textproc/
git-chglog/
20 lines
42 lines

Diff 85834

View Options

head/textproc/git-chglog/Makefile

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines for (n = 0; n < G_N_EVENTREFS; n++) {
break; break;
} }
} }
if (TAILQ_EMPTY(&g_events)) if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events); wakeup(&g_pending_events);
mtx_unlock(&g_eventlock); mtx_unlock(&g_eventlock);
} }
static int struct g_event *
g_post_event_x(g_event_t *func, void *arg, int flag, int wuflag, struct g_event **epp, va_list ap) g_alloc_event(int flag)
jhbUnsubmitted
Not Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage).

jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
impAuthorUnsubmitted
Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that.

I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there...

You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage).

That's a good idea.

imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
jhbUnsubmitted
Not Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that.

I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there...

g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of g_vfs_done:

	if (destroy)
		g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL);

That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?)

Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted.

jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{ {
struct g_event *ep; KASSERT(flag == M_WAITOK || flag == M_NOWAIT,
("Wrong flag to g_alloc_event"));
return (g_malloc(sizeof(struct g_event), flag | M_ZERO));
}
static void
g_post_event_ep_va(g_event_t *func, void *arg, int wuflag,
struct g_event *ep, va_list ap)
{
void *p; void *p;
u_int n; u_int n;
g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 || wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag | M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag; ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) { for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *); p = va_arg(ap, void *);
if (p == NULL) if (p == NULL)
break; break;
g_trace(G_T_TOPOLOGY, " ref %p", p); g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p; ep->ref[n] = p;
} }
KASSERT(p == NULL, ("Too many references to event")); KASSERT(p == NULL, ("Too many references to event"));
ep->func = func; ep->func = func;
ep->arg = arg; ep->arg = arg;
mtx_lock(&g_eventlock); mtx_lock(&g_eventlock);
TSHOLD("GEOM events"); TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events); TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock); mtx_unlock(&g_eventlock);
wakeup(&g_wait_event); wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags |= TDP_GEOM; curthread->td_pflags |= TDP_GEOM;
thread_lock(curthread); thread_lock(curthread);
curthread->td_flags |= TDF_ASTPENDING; curthread->td_flags |= TDF_ASTPENDING;
thread_unlock(curthread); thread_unlock(curthread);
}
void
g_post_event_ep(g_event_t *func, void *arg, struct g_event *ep, ...)
jhbUnsubmitted
Not Done
Inline Actions

Why did you expose wuflag in this API? I would perhaps instead just have a new g_waitfor_event_storage if someone needs the EV_WAKEUP variant so that it mirrors the g_post_event/g_waitfor_event API. The 'waitfor' variant could be added in the future when it is needed.

Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use M_WAITOK to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses M_WAITOK.

jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
impAuthorUnsubmitted
Done
Inline Actions

I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense.

imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
{
va_list ap;
va_start(ap, ep);
g_post_event_ep_va(func, arg, 0, ep, ap);
va_end(ap);
}
static int
g_post_event_x(g_event_t *func, void *arg, int flag, int wuflag, struct g_event **epp, va_list ap)
{
struct g_event *ep;
g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 || wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_alloc_event(flag);
if (ep == NULL)
return (ENOMEM);
if (epp != NULL)
*epp = ep;
g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0); return (0);
} }
int int
g_post_event(g_event_t *func, void *arg, int flag, ...) g_post_event(g_event_t *func, void *arg, int flag, ...)
{ {
va_list ap; va_list ap;
int i; int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines
View Options

head/textproc/git-chglog/distinfo

Show First 20 Lines • Show All 335 Lines • ▼ Show 20 Lines for (n = 0; n < G_N_EVENTREFS; n++) {
break; break;
} }
} }
if (TAILQ_EMPTY(&g_events)) if (TAILQ_EMPTY(&g_events))
wakeup(&g_pending_events); wakeup(&g_pending_events);
mtx_unlock(&g_eventlock); mtx_unlock(&g_eventlock);
} }
static int struct g_event *
g_post_event_x(g_event_t *func, void *arg, int flag, int wuflag, struct g_event **epp, va_list ap) g_alloc_event(int flag)
jhbUnsubmitted
Not Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage).

jhb: Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
impAuthorUnsubmitted
Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that.

I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there...

You could also perhaps forward declare 'struct g_event' in geom.h while leaving it opaque so that you could have static type-checking for the new functions (g_alloc_event, g_free_event, g_post_event_storage).

That's a good idea.

imp: > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
jhbUnsubmitted
Not Done
Inline Actions

Do we need a g_free_event as well? In your case in geom_disk you are going to consume it always, but my other case in g_vfs_done() is optional and I will need to be able to free it in g_vfs_close() when a g_vfs object is freed without having its underlying storage destroyed.

geom_disk just posts the event, it doesn't wait for it. As such, it doesn't need to free it. In other waiters, we already expect them to g_free() the memory. This is no different than that.

I'm unfamiliar with your needs here, though... Can you explain it more, since I can't puzzle out why you'd need it there...

g_vfs won't always post the event is the problem. The event I'm preallocating is for this code at the end of g_vfs_done:

	if (destroy)
		g_post_event(g_vfs_destroy, cp, M_WAITOK, NULL);

That is conditional as it only kicks in if a disk is destroyed out from under a filesystem. If you just unmount a filesystem while the disk is still present, the event won't be used. For that latter case where the event isn't used, I need a way to free the event in g_vfs_close(). Now, perhaps what I can do instead is allocate the event in g_vfs_orphan(), I'm just not sure if it's save to do M_WAITOK in g_vfs_orphan vs pre-allocating in g_vfs_open() where M_WAITOK is definitely safe. (Though it looks like it might be the case that I can sleep in g_vfs_orphan since it is always called with the topology lock held which I think is an sx lock?)

Oh, I think due to the opaque use of void * I was worried you couldn't just use g_free() directly, but I guess it would be ok to use g_free() directly for an event allocated by g_alloc_event(). If you document this in a manpage maybe note that an event allocated by g_alloc_event() can be freed via g_free() if it isn't posted.

jhb: > > Do we need a `g_free_event` as well? In your case in geom_disk you are going to consume it…
{ {
struct g_event *ep; KASSERT(flag == M_WAITOK || flag == M_NOWAIT,
("Wrong flag to g_alloc_event"));
return (g_malloc(sizeof(struct g_event), flag | M_ZERO));
}
static void
g_post_event_ep_va(g_event_t *func, void *arg, int wuflag,
struct g_event *ep, va_list ap)
{
void *p; void *p;
u_int n; u_int n;
g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 || wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_malloc(sizeof *ep, flag | M_ZERO);
if (ep == NULL)
return (ENOMEM);
ep->flag = wuflag; ep->flag = wuflag;
for (n = 0; n < G_N_EVENTREFS; n++) { for (n = 0; n < G_N_EVENTREFS; n++) {
p = va_arg(ap, void *); p = va_arg(ap, void *);
if (p == NULL) if (p == NULL)
break; break;
g_trace(G_T_TOPOLOGY, " ref %p", p); g_trace(G_T_TOPOLOGY, " ref %p", p);
ep->ref[n] = p; ep->ref[n] = p;
} }
KASSERT(p == NULL, ("Too many references to event")); KASSERT(p == NULL, ("Too many references to event"));
ep->func = func; ep->func = func;
ep->arg = arg; ep->arg = arg;
mtx_lock(&g_eventlock); mtx_lock(&g_eventlock);
TSHOLD("GEOM events"); TSHOLD("GEOM events");
TAILQ_INSERT_TAIL(&g_events, ep, events); TAILQ_INSERT_TAIL(&g_events, ep, events);
mtx_unlock(&g_eventlock); mtx_unlock(&g_eventlock);
wakeup(&g_wait_event); wakeup(&g_wait_event);
if (epp != NULL)
*epp = ep;
curthread->td_pflags |= TDP_GEOM; curthread->td_pflags |= TDP_GEOM;
thread_lock(curthread); thread_lock(curthread);
curthread->td_flags |= TDF_ASTPENDING; curthread->td_flags |= TDF_ASTPENDING;
thread_unlock(curthread); thread_unlock(curthread);
}
void
g_post_event_ep(g_event_t *func, void *arg, struct g_event *ep, ...)
jhbUnsubmitted
Not Done
Inline Actions

Why did you expose wuflag in this API? I would perhaps instead just have a new g_waitfor_event_storage if someone needs the EV_WAKEUP variant so that it mirrors the g_post_event/g_waitfor_event API. The 'waitfor' variant could be added in the future when it is needed.

Actually, we probably won't ever need the 'waitfor' variant as if you can sleep, you can likely likely use M_WAITOK to alloc the event in the first place. I'm somewhat surprised 'waitfor' even takes a malloc flag. Indeed, every caller uses M_WAITOK.

jhb: Why did you expose `wuflag` in this API? I would perhaps instead just have a new…
impAuthorUnsubmitted
Done
Inline Actions

I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes sense.

imp: I'll just remove the wuflag. It was mostly because g_post_event_x took it. But your logic makes…
{
va_list ap;
va_start(ap, ep);
g_post_event_ep_va(func, arg, 0, ep, ap);
va_end(ap);
}
static int
g_post_event_x(g_event_t *func, void *arg, int flag, int wuflag, struct g_event **epp, va_list ap)
{
struct g_event *ep;
g_trace(G_T_TOPOLOGY, "g_post_event_x(%p, %p, %d, %d)",
func, arg, flag, wuflag);
KASSERT(wuflag == 0 || wuflag == EV_WAKEUP,
("Wrong wuflag in g_post_event_x(0x%x)", wuflag));
ep = g_alloc_event(flag);
if (ep == NULL)
return (ENOMEM);
if (epp != NULL)
*epp = ep;
g_post_event_ep_va(func, arg, wuflag, ep, ap);
return (0); return (0);
} }
int int
g_post_event(g_event_t *func, void *arg, int flag, ...) g_post_event(g_event_t *func, void *arg, int flag, ...)
{ {
va_list ap; va_list ap;
int i; int i;
▲ Show 20 Lines • Show All 59 Lines • Show Last 20 Lines