Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

###No way to use top() correctly###

No way to use top() correctly

I'm not sure you should include concurrent_stack_top(), because it is unclear how you could use that function safely/usefully. For example, suppose you have an inspector that does this:

int totalCount = 0;
for (size_t i = 0; i != limit; ++i)
{
 MyType *foo = (MyType *) concurrent_stack_top(stack);
 totalCount += foo->count;
 foo->count = 0;
}

The first problem is, how would the inspector thread be able to inspect all the nodes on the stack? If some other thread pushed two nodes before the inspector was able to call top(), then the inspector would miss a node. Same thing if a consumer consumed two nodes in a row. What's most likely to happen is that the inspector would just keep getting the same nodes over and over again, which isn't that useful.

What's worse is if you have a consumer like this:

void consumer(void)
{
 while (1) {
 MyType *foo = (MyType *) concurrent_stack_pop(stack);
 
 do_something_with(foo);
 free(foo);
 }
}

Now, if the inspector used top() to retrieve a node, and then a consumer used pop() to retrieve the same node but called free() on it while the inspector was still using it, then the inspector would be dereferencing freed data.

The only safe way to use top() would be to keep the stack locked while using the top node, and then unlock it when you are finished.

###Same thing with size()###

Same thing with size()

The same reasoning applies to concurrent_stack_size(). If you call that function and get some size back, then by the time you are doing anything with the size, the stack could be any size including 0. So I'm not sure how you could use the returned size for any useful purpose.

###No way to use top() correctly###

I'm not sure you should include concurrent_stack_top(), because it is unclear how you could use that function safely/usefully. For example, suppose you have an inspector that does this:

int totalCount = 0;
for (size_t i = 0; i != limit; ++i)
{
 MyType *foo = (MyType *) concurrent_stack_top(stack);
 totalCount += foo->count;
 foo->count = 0;
}

The first problem is, how would the inspector thread be able to inspect all the nodes on the stack? If some other thread pushed two nodes before the inspector was able to call top(), then the inspector would miss a node. Same thing if a consumer consumed two nodes in a row. What's most likely to happen is that the inspector would just keep getting the same nodes over and over again, which isn't that useful.

What's worse is if you have a consumer like this:

void consumer(void)
{
 while (1) {
 MyType *foo = (MyType *) concurrent_stack_pop(stack);
 
 do_something_with(foo);
 free(foo);
 }
}

Now, if the inspector used top() to retrieve a node, and then a consumer used pop() to retrieve the same node but called free() on it while the inspector was still using it, then the inspector would be dereferencing freed data.

The only safe way to use top() would be to keep the stack locked while using the top node, and then unlock it when you are finished.

###Same thing with size()###

The same reasoning applies to concurrent_stack_size(). If you call that function and get some size back, then by the time you are doing anything with the size, the stack could be any size including 0. So I'm not sure how you could use the returned size for any useful purpose.

No way to use top() correctly

I'm not sure you should include concurrent_stack_top(), because it is unclear how you could use that function safely/usefully. For example, suppose you have an inspector that does this:

int totalCount = 0;
for (size_t i = 0; i != limit; ++i)
{
 MyType *foo = (MyType *) concurrent_stack_top(stack);
 totalCount += foo->count;
 foo->count = 0;
}

The first problem is, how would the inspector thread be able to inspect all the nodes on the stack? If some other thread pushed two nodes before the inspector was able to call top(), then the inspector would miss a node. Same thing if a consumer consumed two nodes in a row. What's most likely to happen is that the inspector would just keep getting the same nodes over and over again, which isn't that useful.

What's worse is if you have a consumer like this:

void consumer(void)
{
 while (1) {
 MyType *foo = (MyType *) concurrent_stack_pop(stack);
 
 do_something_with(foo);
 free(foo);
 }
}

Now, if the inspector used top() to retrieve a node, and then a consumer used pop() to retrieve the same node but called free() on it while the inspector was still using it, then the inspector would be dereferencing freed data.

The only safe way to use top() would be to keep the stack locked while using the top node, and then unlock it when you are finished.

Same thing with size()

The same reasoning applies to concurrent_stack_size(). If you call that function and get some size back, then by the time you are doing anything with the size, the stack could be any size including 0. So I'm not sure how you could use the returned size for any useful purpose.

Source Link
JS1
  • 28.9k
  • 3
  • 41
  • 83

###No way to use top() correctly###

I'm not sure you should include concurrent_stack_top(), because it is unclear how you could use that function safely/usefully. For example, suppose you have an inspector that does this:

int totalCount = 0;
for (size_t i = 0; i != limit; ++i)
{
 MyType *foo = (MyType *) concurrent_stack_top(stack);
 totalCount += foo->count;
 foo->count = 0;
}

The first problem is, how would the inspector thread be able to inspect all the nodes on the stack? If some other thread pushed two nodes before the inspector was able to call top(), then the inspector would miss a node. Same thing if a consumer consumed two nodes in a row. What's most likely to happen is that the inspector would just keep getting the same nodes over and over again, which isn't that useful.

What's worse is if you have a consumer like this:

void consumer(void)
{
 while (1) {
 MyType *foo = (MyType *) concurrent_stack_pop(stack);
 
 do_something_with(foo);
 free(foo);
 }
}

Now, if the inspector used top() to retrieve a node, and then a consumer used pop() to retrieve the same node but called free() on it while the inspector was still using it, then the inspector would be dereferencing freed data.

The only safe way to use top() would be to keep the stack locked while using the top node, and then unlock it when you are finished.

###Same thing with size()###

The same reasoning applies to concurrent_stack_size(). If you call that function and get some size back, then by the time you are doing anything with the size, the stack could be any size including 0. So I'm not sure how you could use the returned size for any useful purpose.

lang-c

AltStyle によって変換されたページ (->オリジナル) /